I want to share a small refactoring example with you.
For whatever reason, I have this code:
IF mv_given_cat_for_selection IS NOT INITIAL.
" take what is given.
r_result = VALUE #( ( sign = wmegc_sign_inclusive
option = wmegc_option_eq
low = mv_given_cat_for_selection ) ).
ELSE.
" Take cat from first/any one line.
r_result = VALUE #( ( sign = 'I'
option = 'EQ'
low = mt_component_data[ 1 ]-s_v2_items-cat ) ).
ENDIF.
Note:
- same thing, two wrintings?! Not so good.
sign = 'I' "no, not so nice
sign = wmegc_sign_inclusive "yes, this I like to use.
- but: might not even matter, as 2 of the 3 lines are duplicates anyway. So let's pull stuff out?!
But first: re-name, so we can understand what the result is:
IF mv_given_cat_for_selection IS NOT INITIAL.
" take what is given.
rt_range_of_cat = VALUE #( ( sign = wmegc_sign_inclusive
option = wmegc_option_eq
low = mv_given_cat_for_selection ) ).
ELSE.
" Take cat from first/any one line.
rt_range_of_cat = VALUE #( ( sign = 'I'
option = 'EQ'
low = mt_component_data[ 1 ]-s_v2_items-cat ) ).
ENDIF.
Note: although in each case we only insert a single line, the result is - has to be - table-like (it's a range / type rseloption ).
Now pull similar lines out of the IF:
DATA ls_range_line LIKE LINE OF rt_range_of_cat.
ls_range_line-sign = wmegc_sign_inclusive.
ls_range_line-option = wmegc_option_eq.
IF mv_given_cat_for_selection IS NOT INITIAL.
" take what is given.
ls_range_line-low = mv_given_cat_for_selection.
ELSE.
" Take cat from first/any one line.
ls_range_line-low = mt_component_data[ 1 ]-s_v2_items-cat.
ENDIF.
INSERT ls_range_line INTO TABLE rt_range_of_cat.
Pull the important part up as far as possible:
DATA ls_range_line LIKE LINE OF rt_range_of_cat.
IF mv_given_cat_for_selection IS NOT INITIAL.
" take what is given.
ls_range_line-low = mv_given_cat_for_selection.
ELSE.
" Take cat from first/any one line.
ls_range_line-low = mt_component_data[ 1 ]-s_v2_items-cat.
ENDIF.
ls_range_line-sign = wmegc_sign_inclusive.
ls_range_line-option = wmegc_option_eq.
INSERT ls_range_line INTO TABLE rt_range_of_cat.
And maybe we can now remove the comments:
DATA ls_range_line LIKE LINE OF rt_range_of_cat.
IF mv_given_cat_for_selection IS NOT INITIAL.
ls_range_line-low = mv_given_cat_for_selection.
ELSE.
ls_range_line-low = mt_component_data[ 1 ]-s_v2_items-cat.
ENDIF.
ls_range_line-sign = wmegc_sign_inclusive.
ls_range_line-option = wmegc_option_eq.
INSERT ls_range_line INTO TABLE rt_range_of_cat.
Just to make that clear: it's the whole method:
METHOD hu_sel_build_range_for_cat.
DATA ls_range_line LIKE LINE OF rt_range_of_cat.
IF mv_given_cat_for_selection IS NOT INITIAL.
ls_range_line-low = mv_given_cat_for_selection.
ELSE.
ls_range_line-low = mt_component_data[ 1 ]-s_v2_items-cat.
ENDIF.
ls_range_line-sign = wmegc_sign_inclusive.
ls_range_line-option = wmegc_option_eq.
INSERT ls_range_line INTO TABLE rt_range_of_cat.
ENDMETHOD.
Finally: in my codebase I cant do that, but I can do it for the blog: different naming / dropping prefixes:
METHOD hu_sel_build_range_for_cat.
DATA range_line LIKE LINE OF result_range_of_cat.
IF given_cat_for_selection IS NOT INITIAL.
range_line-low = given_cat_for_selection.
ELSE.
range_line-low = component_data[ 1 ]-s_v2_items-cat.
ENDIF.
range_line-sign = wmegc_sign_inclusive.
range_line-option = wmegc_option_eq.
INSERT range_line INTO TABLE result_range_of_cat.
ENDMETHOD.
What I note:
- went away from "new" ABAP back to very traditional one.
- didn't realy save on lines: 11 (with 2 commets) vs. 10 (without comments).
- main improvement was: keeping the IF part smal and dense.
What do you think? Was it an improvement?
How would you write that code?
best
Joachim
