SAP / styleguides

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

Misleading Method Name vs Built-in Function #250

Open lucasborin opened 3 years ago

lucasborin commented 3 years ago

In a short dialogue about the self-reference rule (Code Pal, Clean ABAP), it came to a strange scenario where a class method is named as a built-in function.

For instance:

CLASS person DEFINITION.
  PUBLIC SECTION.
    METHODS is_valid RETURNING VALUE(result) TYPE abap_bool.
  PROTECTED SECTION.
    METHODS strlen RETURNING VALUE(result) TYPE i.    
  PRIVATE SECTION.
    DATA name TYPE string.
ENDCLASS.

If you want to call the built-in function strlen, you are not allowed anymore because you "replaced" it:

  METHOD is_valid.
    result = xsdbool( strlen( name ) > 1 ).  "<---- syntax error
  ENDMETHOD.

But, you can:

  METHOD is_valid.
    result = xsdbool( strlen( ) > 1 ).
    result = xsdbool( me->strlen( ) > 1 ).
  ENDMETHOD.

I haven't found anything on this matter in the style guide, and I will open a pull request discouraging the reuse of the built-in function names like the line_exists, lines, line_index, strlen, etc. In parallel, I would like to see your opinion about it.

Do you consider it a bad practice? Do you see it needed sometimes?

lucasborin commented 3 years ago

image

Ref: Obscuring with Methods

jordao76 commented 2 years ago

This is all well and good, but does the compiler or code pal warn you that you're obscuring a built-in function? If not, you should be aware of all existing built-in functions.

There's also the problem of future built-in functions. Should be rare, but we should not retroactively warn users about new built-in functions introduced in the language. In the end, if the class is well designed and tested and uses a method with the name of a built-in function, then it was not intended to use the built-in function itself and all should be good.

larshp commented 2 years ago

here, https://rules.abaplint.org/method_overwrites_builtin/

ConjuringCoffee commented 1 year ago

Can't this issue be closed? #251 from 2022 covered this matter sufficiently in my opinion.