SAP / styleguides

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

use/definining of public ALIASES #176

Open larshp opened 3 years ago

larshp commented 3 years ago

Hi, Any thoughts on public ALIASES?

Personally, I'm leaning towards it being bad in all cases, it's either another indirection or something wrong in the Object Oriented design.

fabianlupa commented 3 years ago

Just an example for discussion:

In this case one approach might be to alias the lock methods to "make them available" to the caller more easily? Instead of creating a new method that just wrap the interface ones.

larshp commented 3 years ago

If there is a class CL_CUSTOMER and CL_ORDER both implementing IF_LOCKABLE, then there is a possibility that these are handled by the same "lock manager" which casts the objects to IF_LOCKABLE, ie. no aliases are needed.

If locking is handled at a non-generic level, then the locking should be at a non-generic level

🤷‍♂️

jordao76 commented 3 years ago

One use for aliasing is when extracting an interface from an already existing class. Some of the method definitions of the class might migrate to the interface, and aliases for them will not only make it easier for the class to implement them since the class implementation area won't need to change but will also make sure that clients of the old methods do not break.

Aliases can also give more meaningful names for generic interface method names in the context of a specific class implementation, if only to make the code within the class or its unit tests more readable.

larshp commented 3 years ago

NEW zcl_bar( )->interface_method( )., yea, its convenient, but I'd argue that its not proper object oriented if the interface_method can be accessed outside a interface reference

naming, yea, but the method(or type/attribute) should be properly named in the first place, renaming it makes another indirection, which is more difficult to understand IMHO

pokrakam commented 3 years ago

I'm also not the biggest fan of public aliases. IMHO they should be used sparingly and with purpose, not for convenience.

AS @jordao76 mentioned, aliases are useful for refactoring methods into an interface to ensure that existing code doesn't break. But this should be short-term and the direct users of the class should be refactored. The use of aliases should be seen as a TODO here, because we want to decouple them.

I'm OK with private aliases for internal readability, but they still suffer from the problem that aliases make refactoring more work. Refactoring tools such as ADT don't touch them (IMO correct behaviour).

Consider an example that begs for refactoring:

ALIASES check FOR zif_myobj~check
...
METHOD do_something. 
  IF check( ). 

If I do the good Boy Scout thing and rename check, all non-aliased usage of the interface are updated, but aliased code stays as is and we end up with:

ALIASES check FOR zif_myobj~date_in_validity_period.
...
METHOD do_something. 
  IF check( ). 

It's not just a matter of more work, I may be unaware of this alias if I do this refactoring while working in a different implementing class. ADT is pretty good at refactoring many implementers in one go, and unit tests will still report that everything is green.

pokrakam commented 3 years ago

That said, public aliases are useful for nested interfaces. Should also be used with caution but sometimes good for proper interface segregation:

INTERFACE lif_reader.
  METHODS read.
ENDINTERFACE.

INTERFACE lif_crud.
  INTERFACES lif_reader.
  ALIASES read FOR lif_reader~read.

  METHODS:
    create,
    update,
    delete.
ENDINTERFACE.