SAP / styleguides

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

"Align parameters" versus "Prefer functional to procedural calls" #258

Closed cshbicos closed 2 years ago

cshbicos commented 2 years ago

While I agree that functional calls to methods are better due to chaining and also avoid superfluous text, I found that the loss of parameter alignment (section "Align parameters") through Pretty Printer usually tips the balance in favor of procedural calls.

Every time the maximum parameter length is changed, it is annoying to add/delete white spaces to align again. Especially with larger refactoring, this can be quite time consuming.

Maybe I've missed some magic trick, but running pretty print against

CALL METHOD modify->update
EXPORTING
    node  = /dirty/my_bo_c=>node-item
key = item->key
                 data = item
changed_fields = changed_fields.

will make the parameters align again beautifully, while running pretty print against this

modify->update( node = /clean/my_bo_c=>node-item
key = item->key
     data = item
  changed_fields = changed_fields ).

keeps the parameter list all unwieldy and jagged.

Personally, I prefer a clean indentation provided & maintained automatically - even it that means taking a hit on missing out on chaining.

nununo commented 2 years ago

I would argue that it is Pretty printer that must evolve and not you that must be stuck in the past. While that doesn't happen, if I'm not mistaken, if you align them manually, pretty printer will not mess with them. So, while Pretty printer is not fixed, I'd rather do it manually than reverting to CALL METHOD.

fabianlupa commented 2 years ago

Pretty printer apparently does now support this on a recent enough release, at least that's how I understood the patch notes:

https://help.sap.com/viewer/4726775c8bfc483abb210252604515b2/Cloud/en-US/a2506f3443404c34b8db478f4366c6f6.html#loio6cc46725cb9b47a5963ee563c5c88294__section_pretty_printer

https://help.sap.com/viewer/5371047f1273405bb46725a417f95433/Cloud/en-US/81717979de0243658237fbe2be5a2c6e.html

s7oev commented 2 years ago

@flaiker - just tested on a recent release, can confirm it's exactly this.

Before pretty printer: image After pretty printer: image

cshbicos commented 2 years ago

That's great news about that new Pretty Printer option - sounds like having the cake and eating it too!

Thanks for pointing it out @flaiker!