search

SAP / styleguides

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

Don't declare inline in optional branches #272

Closed larshp closed 1 year ago

larshp commented 2 years ago

https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#dont-declare-inline-in-optional-branches

In this example a variable is declared in an optional branch,

IF foo = bar.
  LOOP AT tab INTO DATA(sdf).
  ENDLOOP.
ENDLOOP.

according to the rule description, this is not good, and should be changed to

DATA sdf LIKE LINE OF tab.
IF foo = bar.
  LOOP AT tab INTO sdf.
  ENDLOOP.
ENDLOOP.

? effectively a lot of inline declarations should not be used, ever?

fabianlupa commented 2 years ago

I always followed this rule: If explicit declaration then move to top, if inline it's fine within nested branches as long as you don't use it outside that branch which would technically be possible. Not sure if that's the best approach.

larshp commented 2 years ago

yea, I agree, but its strictly not what the description conveys

it should be something like "Dont use inline in unrelated branches" or "Only use inline within branch where its defined"

https://github.com/abaplint/abaplint/issues/2522

N2oB6n-SAP commented 1 year ago

I agree. An additional example would be nice. It should outline that this is perfectly fine if the variable is only used in the "nesting scope". Both an ELSE branch as well as an access at the top-level of the method body would be considered "outside" of the nesting scope of the IF branch where it is introduced.

Jelena-P commented 1 year ago

I'm with @fabianlupa on this: if it's used in multiple branches, the declare explicitly. It's OK to declare inline if it's only used in the same branch and not anywhere else. Text could be updated accordingly, otherwise as @larshp pointed out, it sounds like no one will be able to use inline declarations in like 50% of cases.