styleguides icon indicating copy to clipboard operation
styleguides copied to clipboard

Chained Methods/Constants

Open lucasborin opened this issue 3 years ago • 5 comments

In short, we should not chain up-front variable declarations. But, how about METHODS, CLASS-METHODS, TYPES, and CONSTANTS?

For instance:

METHODS:
  get_name,
  get_age,  
  set_name,
  set_age. 
CONSTANTS:
  min_name_size TYPE i VALUE 3,
  min_age_allowed TYPE i VALUE 18.

lucasborin avatar Oct 22 '21 14:10 lucasborin

Personally I follow the same principle and only chain closely related entities if it aids readability (i.e. short declarations). In practice I don't chain often, and usually break up the generated declarations that quick-fix insists on chaining. In Unit Tests is where chaining is probably the most useful:

"OK
METHODS setup. 

METHODS: 
  doc_type_a_is_accepted FOR TESTING,
  doc_type_b_fails FOR TESTING.

METHODS: 
  positive_qty_adds_item FOR TESTING,
  zero_qty_no_item FOR TESTING,
  negative_qty_raises_exception FOR TESTING.

I prefer horizontal density, and find wading through pages of long vertical declarations becomes unhelpful. We should definitely not chain when the METHODS part is more than 10-20 lines or so away from the declaration as you have no idea whether it's a static or public component or even an alias if you navigate there from somewhere else and have to hunt for the start of the chain.

"anti-pattern
    METHODS: 
      add
        IMPORTING
          num1 TYPE i
          num2 TYPE i
        RETURNING
          result TYPE i
        RAISING
          zcx_not_a_number,
      subtract
        IMPORTING
          num1 TYPE i
          num2 TYPE i
        RETURNING
          result TYPE i
        RAISING
          zcx_not_a_number,
      multiply
        IMPORTING
          num1 TYPE i
          num2 TYPE i
        RETURNING
          result TYPE i
        RAISING
          zcx_not_a_number,
      divide
        IMPORTING
          num1 TYPE i
          num2 TYPE i
        RETURNING
          result TYPE i
        RAISING
          zcx_not_a_number
          zcx_not_a_multiple.

For a good antipattern, imagine class CL_WB2_REBATE_SETTLEMENT with chained declarations...

pokrakam avatar Oct 24 '21 10:10 pokrakam

Curiously enough, the identifiers are in plural mode: METHODS and CONSTANTS for example. They imply chaining. I always find it awkward to find a lone method declared with METHODS.... Either way I think your idea of grouping related methods to be most ideal.

jordao76 avatar May 22 '22 00:05 jordao76

Since the inline declarations reduce the amount of necessary up-front declarations immensely I see no disadvantages in chaining declarations. A grouping of the declarations using comments or empty lines in between seems to me also a proper way to do that. What do you think? Furthermore I don't understand the statment of complicating the reformatting in chapter "Do not chain up-front declarations" as the pretty printer handles this nicely.

JSB-Vienna avatar Jul 17 '22 10:07 JSB-Vienna

Do you handle the use of BEGIN OF & END OF differently? Personally, I prefer to chain these types of declarations. They definitely belong together, so chaining them makes sense to me.

CONSTANTS:
  BEGIN OF languages,
    german  TYPE spras VALUE 'D',
    english TYPE spras VALUE 'E',
  END OF languages.

ConjuringCoffee avatar Jul 22 '22 10:07 ConjuringCoffee

I only use chaining with TYPES. For all others (DATA, CONSTANTS, METHODS) I never use a the colon. Reasons: much easier refactoring, when deleting, copying or moving things.

Examples:

METHODS: 
  one,
  two,
  three.

Deleting or moving method ONE leads to

  • add METHODS to mehod ONE or the rest of methods (depends on if METHODS stands in single line or directly in front of ONE).
  • adjust comma for method ONE when moving

Otherwise declarations can easily be deleted using CTRL+D or moving lines using ALT+ <cursor up/down>

Ennowulff avatar May 15 '23 15:05 Ennowulff