Functional operator COND vs IF?

[SuhaSaha]

I like using ABAP functional operators in my daily coding, but sometimes i question the readability (CLEANliness) of the such coding blocks.

What i currently have:

R_RESULT 
= cond #( 
    when I_PARAM is initial
         and UTILITY->FOO( ) <> IF_ENUM=>CONSTANT-UNDEFINED
      then throw CX_ERROR( message E004(MSG_CLS) )
    when I_PARAM is not initial
      then cond #( when I_SOME_FLAG = ABAP_TRUE
                     then throw CX_ERROR( message E006(MSG_CLS) with I_PARAM )
                   else ABAP_TRUE ) ).

I could rewrite the code in the old verbose way using IFs:

if I_PARAM is initial
   and UTILITY->FOO( ) <> IF_ENUM=>CONSTANT-UNDEFINED.
  raise exception type CX_ERROR message E004(MSG_CLS).
elseif I_PARAM is not initial.
  if I_SOME_FLAG = ABAP_TRUE.
    raise exception type CX_ERROR message E006(MSG_CLS) with I_PARAM.
  else.
    R_RESULT = ABAP_TRUE.
  endif.
endif.

IMO, the first code snippet is "cleaner" because from the first glance i can see this COND function block returns me only R_RESULT. In the second snippet this is not the case.

I would like to hear your thoughts about it.

BR,

Suhas

[Sandra_Rossi]

I use constructor expressions exactly for the reason you mention: "from the first glance I can see this COND function block returns me only R_RESULT" - AND to avoid confusions with all intermediate variables.

The potential complexity of a construction expression is often criticized, but often because developers are not used to work with them yet. I don't hesitate to have a complex expression to avoid all intermediate variables (and to show that the goal is to initialize only one variable). An automated unit test should help understand what a complex construction expression does, if it's not clear to a developer. Of course, I think there's a limit in the complexity one shouldn't reach but I can't tell how to define it.

PS: maybe your example is not the best one to illustrate your question: I would keep the old way because the conditions are only to throw exceptions, and then the returned value is only to show that there's no exception. A better example would be R_RESULT = ABAP_FALSE instead of the two RAISE EXCEPTION TYPE.

EDIT: following your comment and Jelena's comment, I realize that I didn't read well the condition, that you assign either false or true or throw an exception. Maybe a better style would be then to focus first on the values you assign because it's all about a constructor expression, and exception is thrown only in other cases i.e. WHEN ... THEN abap_true WHEN ... THEN abap_false ELSE THROW exception.

[SuhaSaha]

I would keep the old way because the conditions are only to throw exceptions, and then the returned value is only to show that there's no exception.

This was the reason why i questioned the functional expression. I will extract the exceptions into a (private) pre-check method & then determine R_RESULT.

[DoanManhQuynh]

I think Throw exception inside COND is to determine the exception case in ELSE, the main purpose is in THEN so what you are doing is opposite with it natural. in your example it maybe "cleaner" to see it in classic way because IF work as branch condition, you can do whatever you want after that. with COND it should clear about its purpose: to build return value (R_RESULT = true or false) or to determine the exception.

[Jelena]

Nice to see questions like this pop up at least once in a while. 🙂

Just my $0.02. I think in this case we are in the style territory where instead of right or wrong it's more about personal preference than anything else.

With that disclaimer, in this particular case I prefer IF... version because I can follow it more easily. And it's not because I'm an ABAPosaur who dislikes COND but because, as others noted, here the exceptions are mixed in with the value assignment. If it wasn't the case then COND option could've been better.

As a side note, in either case it bugs me that if i_param IS INITIAL but that second AND UTILITY... condition is not fulfilled the whole IF statement will be just skipped. Simply by looking at the code, I can't tell if it's intentional or an oversight. Also I'm usually not a fan of many nested IFs but in this particular case I feel like breaking apart the first IF... AND... into separate statements, which will allow to change ELSEIF... into simply ELSE. This cuts down on verbosity and is more convenient from the debugging perspective. As Sandra correctly noted though, to an automated test this is not an issue.