Small refactoring towards cleaner code: keep the IF small

[joachimrees1]

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