Usage of CHECK as the first statement inside LOOP

[suynwa]

I know that the topic has been addressed in the section Avoid CHECK in other positions. I'm raising this issue as a follow-up to this Code-Pal issue.

Option 1: Using CHECK as the first statement

loop at COMPANIES assigning field-symbol(<COMPANY>).
  check _IS_COMPANY_IN_EU( <COMPANY> ).
  ...
endloop.

Option 2: Using CONTINUE to skip the iteration

loop at COMPANIES assigning field-symbol(<COMPANY>).
  if not _IS_COMPANY_IN_EU( <COMPANY> ).
    continue.
  endif.
  ...
endloop.

Option 3: Using IF instead of CHECK

loop at COMPANIES assigning field-symbol(<COMPANY>).
  if _IS_COMPANY_IN_EU( <COMPANY> ).
  ...
  endif.
endloop.

It is stated in the styleguide and i quote:

Do not use CHECK outside of the initialization section of a method. The statement behaves differently in different positions and may lead to unclear, unexpected effects.

Why isn't the usage of CHECK as the "initial" condition inside a LOOP considered to be clean?

[pokrakam]

In my opinion I think the guidelines are fine as is. The "Clean" principle should also apply to the guidelines, and IMHO allowing CHECK here and here but not there and not there complicates things. Right now we have one rule and one exception.

I also feel that exiting at the beginning of a loop is a code smell. Here the loop is doing two things, applying a complex filter and processing, and in most cases these functions can be separated. Some ideas around your example: LOOP at filter_eu( companies ) ...

Or for complex or varying conditions, drop LOOP and encapsulate the scope in an iterator instead, something like:

companies->set_filter( eu = abap_true ).
data(company) = companies->get_first( ). 
WHILE company IS BOUND.     "Using ref instead of field symbol, but that's another debate :)
  ...
  company = companies->get_next( ).
ENDWHILE.

[suynwa]

Hi Mike,

Thanks for the response. But unfortunately, i beg to differ with you on this occasion 😉

The "Clean" principle should also apply to the guidelines, and IMHO allowing CHECK here and here but not there and not there complicates things. Right now we have one rule and one exception.

True that! My suggestion is to allow CHECK as the initial condition inside a processing block - be it a procedure or an iteration. The behavior is also well-documented. Somehow i have the feeling that this "Clean" principle is to prevent the developers, who don't RTFM, from making mistakes.

LOOP at filter_eu( companies ) ...

How would the FILTER_EU method look like then?

method FILTER_EU_COMPANIES.

  loop at I_COMPANIES assigning field-symbol(<COMPANY>).
    check IS_IN_EU( <COMPANY> ). " Use CHECK as the initial condition
    insert <COMPANY> into table R_RESULT.
  endloop.

endmethod.

OR

method FILTER_EU_COMPANIES.

  loop at I_COMPANIES assigning field-symbol(<COMPANY>).
    if IS_IN_EU( <COMPANY> ).
      insert <COMPANY> into table R_RESULT.
    endif.
  endloop.

endmethod.

drop LOOP and encapsulate the scope in an iterator instead...

I am not a big fan of iterators, when the same thing can be achieved using LOOP. I prefer to K.I.S.S 😊

BR, Suhas

[ruthiel]

About this I totally agree with @pokrakam . The blocks are encapsulated and easy to read without the necessity of comments. The code says it all when the method names are well chosen.

[pokrakam]

@suynwa In my example my intention was that the is_in_eu method of your original example is refactored and contains the appropriate logic directly.

METHOD filter_eu. 
  SELECT ... 
    FROM @companies INNER JOIN t001 ON ...
    WHERE ...
    INTO TABLE @result. 
ENDMETHOD.

METHOD is_in_eu. 
  result = xsdbool( filter_eu( value #( ( company ) ) IS NOT INITIAL ).
ENDMETHOD.

Sometimes Clean Code does become more complex, SoC is not always synonymous with simplicity.

I like using iterators and collections when the looping or access logic becomes more complex than a simple key or condition. I also agree with KISS, they should be simple, often mine are very small local classes with 2-3 methods and sometimes only <5 statements in total. The point is mainly to simplify the calling code.

Your example may not necessarily be appropriate for an iterator, it really depends on the overall usage and was just an idea. I would typically use an iterator when we have several different ways of looping and/or filtering repeated in different parts of code, or when the looping is complex (e.g. traversing a tree or linked lists).