SAP / styleguides

This repository provides SAP style guides for coding and coding-related topics.
Other
1.68k stars 445 forks source link

Prefer 'if not method_call( )' to 'if method_call( ) = abap_false' #236

Closed PeregrinTooc closed 2 years ago

PeregrinTooc commented 3 years ago

In this issue in codepal for abap the discussion is which of the following is more in the spirit of clean abap:

if not condition_is_fulfilled( ).
...
endif.

or

if condition_is_fulfilled( ) = abap_false.
...
endif.
lucasborin commented 3 years ago

Same question, but for the positive conditions:

if condition_is_fulfilled( ).
...
endif.

or

if condition_is_fulfilled( ) = abap_true.
...
endif.

?

jdgx commented 3 years ago

Hi all,

As mentioned in the codepal discussion my vote goes in both cases to the 'operatorless' version

if [not] condition_is_fulfilled( ).
...
endif.

not just because it's shorter, but mainly because it is closer to natural language and hence easier to read and easier to understand.

Best, Jonathan

ConjuringCoffee commented 3 years ago

I agree with @jdgx.

pokrakam commented 3 years ago

I agree with the shorter syntax, but I think there should be a caveat that omitting the comparison is just a shorthand form for IS NOT INITIAL and care must be taken to ensure it really does return an ABAP Boolean space/X.

Otherwise we could run into trouble:

METHOD is_true. 
  result = 'N'. 
ENDMETHOD. 
...
IF is_true( ).   "==> True!
jdgx commented 3 years ago

@pokrakam Good point, I didn't even know that this was a shorthand for is not initial. This could be a case for another recommendation (or codepal check): Don't use this shorthand for non-"abap_bool"-ean methods.

But back to the original issue of real boolean methods: as far as I see we all agree that that omitting the operator is the preferred option. How shall we go on? Has anybody already thought about the exact wording? Should we discuss this here or should I come up with something and create a pull request?

jdgx commented 3 years ago

With some delay, the pull request is finally there. Let me know if you have any feedback.