关于CRM product API的一些代码审查建议

GET_SALES_DATA
Issue1:

Issue2:

GET_HEADER_WITH_DESC_BY_GUIDS
Issue3:
still remember the cyclomatic complexity concept in my S.O.L.I.D session?
Instead of using

METHODS:
IF < condition is fulfilled>
   DO SOMETHING.
ENDIF.
ENDMETHOD.
use code below to AVOID IF :
METHODS:
CHECK <condition is NOT fulfilled>
DO SOMETHING.
ENDMETHOD.

get_required_select_list - DRY!!
Issue4:

The improved solution to avoid SO MANY IF: execute the following report in AG9

REPORT ztest_wei.

TYPES: BEGIN OF ty_select_list_rule,
         flag_name TYPE string,
         operation TYPE string,
       END OF ty_select_list_rule.

TYPES: tt_select_list_rule TYPE STANDARD TABLE OF ty_select_list_rule WITH KEY flag_name.

START-OF-SELECTION.

  DATA: ls_rule TYPE ty_select_list_rule,
        lt_rule TYPE tt_select_list_rule.

  ls_rule = VALUE #( flag_name = 'is_prod_shtext_required' operation = ', \_ProductShortText-ProductName as ShortText' ).
  APPEND ls_rule TO lt_rule.

  ls_rule = VALUE #( flag_name = 'is_material_type_text_required' operation = ', \_MaterialTypeText-MaterialTypeName as MaterialTypeText' ).
  APPEND ls_rule TO lt_rule.

  ls_rule = VALUE #( flag_name = 'is_base_unit_text_required' operation = ', \_BaseUnitText-UnitOfMeasureName as BaseUnitText' ).
  APPEND ls_rule TO lt_rule.

  DATA(ls_data) = VALUE crms4s_required_fields( is_prod_shtext_required = 'X' is_base_unit_text_required = 'X' ).
  DATA(lv_select_list) = CONV string( 'product_h~*' ).

  PERFORM get_required_select_list USING ls_data CHANGING lv_select_list.

  WRITE:/ lv_select_list.

FORM get_required_select_list USING is_required_fields TYPE crms4s_required_fields
        CHANGING ev_select_list TYPE string.
  DO.
    ASSIGN COMPONENT sy-index OF STRUCTURE is_required_fields TO FIELD-SYMBOL(<status>).

    IF sy-subrc = 0 AND <status> = abap_true.
      ev_select_list = ev_select_list && lt_rule[ sy-index ]-operation.
    ELSE.
      EXIT.
    ENDIF.
  ENDDO.

ENDFORM.

The new implementation only has 9 lines and IFIFIFIFIFIF…IF is avoided:

Issue5:
Is this AS really necessary?

Issue6:
when to add the error handling in TODO?

Update on 2017-02-17
Issue1

Issue2
这个method的signature设计地有点confusing,
输入参数里的UOM_DESC和输出参数的UOM_DESC是配对的，这里给我一个错觉，如果IV_UOM_DESC_REQUIRED = TRUE, 那么输出ET_UOM_DESC. 反过来，如果IV_UOM_DESC_REQUIRED = FALSE, 是不是就不输出东西了？

要获取更多Jerry的原创文章，请关注公众号"汪子熙":