Indentation of parameters

[sbcgua]

Hi! Great document. Clean, searchable, with examples, very good job. Thanks ! I'd say 80% are to "immediate compulsory application" but there are couple of things I disagree (this and a separate issue I'll create in a moment).

I'd strongly disagree with "Keep parameters behind the call", "If you break, indent parameters under the call" and thus indirectly with most of examples in "Formatting sections". The most concentrated example I would definitely avoid is in "Indent in-line declarations like method calls" section - multilevel placement of parameters. This is imho exactly the opposite to "optimize for reading" intention :)

This way of alignment depends on variable/callee length and thus places in unpredictable place and thus directly cripples the readability.

XXXX = XXXXXXXXXXXXXXX ( YYY    = ZZZ 
                         YYYYYY = ZZZZZZZ ).

versus

XXXX = XXXXXXXXXXXXXXX ( 
  YYY    = ZZZ 
  YYYYYY = ZZZZZZZ ).

In the second case eye expects parameters in a familiar place and thus consumes it faster. This way "keeps the important things in a single (left) column"

For the reference: The subject is very well clarified in this video (staring at 10:30 ~ 23:00 with some very bright examples at ~21:00+ ).

P.S. BTW the rest of the mentioned video also worth integrating :) E.g. using "get" in method names.

[HrFlorianHoffmann]

Some reasoning behind the advice.

As Kevlin Henney also points out in the video you linked, we perceive objects as belonging together if they are visually close.

In the following common case, indenting the parameters at line start visually groups them with the receiving parameter, which is misleading, as they more belong to the method name.

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = xxxxx(
  xxx  = xxx
  xxxx = xxxx ).

This is not unique to ABAP, and looks misleading in other programming languages as well:

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = xxxxx(
  xxx, xxxx);

The problem fixes itself if there are more or longer parameters:

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = xxxxx(
  xxx, xxxx, xxxxx, xxxxxxxxxx, xxxxxxxxx);

If that is not the case, other languages tend to simply not break in this case, probably even ignoring the adviced max line length:

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = xxxxx(xxx, xxxx);

For ABAP, this is not really an option, as stringing multiple parameters in one line becomes not only very long but also obscures where one parameter ends and the next starts. For ABAP, an alternative solution could be adding a line break, instead of removing one:

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx =
  xxxxx(
    xxx  = xxx
    xxxx = xxxx ).

[sbcgua]

This one is totally ok :)

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx =
  xxxxx(
    xxx  = xxx
    xxxx = xxxx ).

But then why not to recommend it ?

Because the current examples in the section are exactly negatively addressed by Kevlin at ~21:15 - "many line of attention".

P.S. Just as an example from other languages e.g. object paramteres in JS also tend to be:

myfunc({
  url: '...',
  port: 8080,
  auth: ...
});

[filak-sap]

I prefer:

xxx = xxxxx(
  xxx = xxx
  xxxxx = xxxxx
).

because without the bracket on new line, the passed parameters looks like variable assignment statements and that confuses me when doing reviews in GitHub.

BTW: this version lacks of visual separator (if only we could separate parameters by comma):

xxx = xxxxx( xxx = xxx xxxx = xxxx ).

BTW 2: not aligned equal signs are not such a big problem, if you order parameters by length:

xxx = xxxxx(
  xx = xx
  xxx = xxx
  xxxx = xxxx
  xxxxx = xxxxx
).

[tricktresor]

I also prefer having

XXXX = XXXXXXXXXXXXXXX ( 
  YYY    = ZZZ 
  YYYYYY = ZZZZZZZ ).

because of the following reason: Refactoring. It often happens, that I do one of the following things:

• adding or removing inline declaration
• renaming methods or variable names

This causes to break the carefully indented code

XXXX = XXXXXXXXXXXXXXX ( YYY    = ZZZ 
                         YYYYYY = ZZZZZZZ ).

to

DATA(XXXX )= yy ( YYY    = ZZZ 
                         YYYYYY = ZZZZZZZ ).

while it does not break the first mentioned variant.

[bennyPatto]

+1 for

xxx = xxxxx(
  xxx = xxx
  xxxxx = xxxxx
).

Ending bracket on separate line aligned with where the bracket is opened is important when nesting.

xxx = xxxxx(
  xxx = xxx( 
    xxx = xxx
    xxx = xxx
  )
  xxxxx = xxxxx
).

I think it is also important to highlight in this discussion the use monadic methods if you have control to do so in order to clean the code. This makes single line method calls much cleaner (although there is a trade-off for method api - such as mandatory parameters if using structures)

[nomssi]

I do not like parens on their own line, then parens tend to feel lonely

[obr-sap]

+1 for two arguments made here:

(A) Refactoring: Robustness against refactoring means that any trailing indentation is bound to fail as long as it depends on names that could be refactored. (B) Code Reviews: Irrelevant diffs need to be minimized for code reviews (and version history). This means that the closing parenthesis should not appear behind a parameter if a further parameter might be added in the future.

On top of that, it is important to keep the lines short in order to be able to conduct meaningful three-way merges.

Based on this reasoning, the sensible choice is

xxx = xxxxx(
  xxx = xxx( 
    xxx = xxx
    xxx = xxx
  )
  xxxxx = xxxxx
).

[nununo]

(B) Code Reviews: Irrelevant diffs need to be minimized for code reviews (and version history). This means that the closing parenthesis should not appear behind a parameter if a further parameter might be added in the future.

The above argument is enough for me to vouch for this approach:

xxx = xxxxx(
  xxx = xxx( 
    xxx = xxx
    xxx = xxx
  )
  xxxxx = xxxxx
).

[s7oev]

Out of curiosity: is there a commonly used style in any other (popular) language that recommends aligned equal signs? Usually when I've seen Python/Java/Scala/JS code, the approach is always either:

myVar = myFunction(
    myParam1 = "myVal1",
    myLongerNamedParam2 = 42
);

or

myVar = myFunction(
    myParam1 = "myVal1",
    myLongerNamedParam2 = 42);

I haven't really seen:

(screenshot as I can't get the markdown to render the amount of spaces properly for whatever reason 😶)

[nununo]

I have never seen any other language which indents variable and attribute declarations like ABAP. Besides, Clean Code explicitly advises against vertical alignment of variables and its types. I guess Clean ABAP, being inspired in Clean Code, should follow that advice.

[fabianlupa]

Note that non-aligned equal signs would be inconsistent currently, as you cannot turn off the equal sign alignment for function calls and non-functional method calls in the pretty printer settings. Also there's more horizontal alignment like in method definitions. So it's not really a recommendation in ABAP but rather enforced by the tooling. (And as a result of that I'd rather be consistent than have mixed styles.)

[obr-sap]

Shouldn't the pretty printer follow clean code guidance instead of vice versa? Once we agree on what formatting is best, we'd have some leverage on the pretty printer definition...

[fabianlupa]

Shouldn't the pretty printer follow clean code guidance instead of vice versa? Once we agree on what formatting is best, we'd have some leverage on the pretty printer definition...

Oh for sure. This might however cause a release based differentiation in the guideline (e.g. don't horizontally align at all on > 7.56 where pretty printer is adjusted, do always horizontally align on any lower release where it's not for consistency), which seems a bit ridiculous talking about code formatting.

[pokrakam]

I don't mind horizontally aligned =s, but then I try to keep names of a similar length. If there's an outlier then it does decrease readablilty.

FWIW, in 7.55 (or possibly earlier) the ADT formatter gained the capability to do vertical formatting and I'm a fan.