Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Use CHANGING sparingly, where suited] Is it ok to use IMPORTING REF instead of CHANGING? #331

Closed
Yrfo opened this issue Jun 22, 2023 · 7 comments
Labels
Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap

Comments

@Yrfo
Copy link

Yrfo commented Jun 22, 2023

Relevant sections of the style guide
Use CHANGING sparingly, where suited

Question
I recently found that I really hate CHANGING parameters, mainly because changing forces to type it with interface parameter name during method call. I got the idea that I even can use importing by reference instead of using changing. But I'm curious if it is ok.

So here is example of use of both variants:

"definition

METHODS example_of_calls.
METHODS change_by_changing
  CHANGING interface_parameter_name TYPE string.
METHODS change_by_ref
  IMPORTING interface_parameter_name TYPE REF TO string.

"implementation

METHOD change_by_changing.
  interface_parameter_name = interface_parameter_name && |added|.
ENDMETHOD.

METHOD change_by_ref.
  interface_parameter_name->* = interface_parameter_name->* && |added|.
ENDMETHOD.

METHOD example_of_calls.
  DATA(variable) = |not initial|.

  change_by_changing( CHANGING interface_parameter_name = variable ).
  change_by_ref( REF #( variable ) ).
ENDMETHOD.

Advantage of the ref that calling is simpler and possible easier refactor for renaming interface parameters.
But disadvantage is that inside of method need to work with ref. Both the advantage and the disadvantage are not big, so probably it is personal. What do you think?

Also another question comes from that discussion.
For example variable is already reference (e.g. class instance) and method changes the content of the variable (e.g. even via it's methods). Can be that variable passed as importing, or should go to changing? and why?

@Yrfo Yrfo added Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap labels Jun 22, 2023
@Yrfo Yrfo changed the title [Use CHANGING sparingly, where suited] Is it ok to use import REF instead of CHANGING [Use CHANGING sparingly, where suited] Is it ok to use IMPORTING REF instead of CHANGING Jun 22, 2023
@Yrfo Yrfo changed the title [Use CHANGING sparingly, where suited] Is it ok to use IMPORTING REF instead of CHANGING [Use CHANGING sparingly, where suited] Is it ok to use IMPORTING REF instead of CHANGING? Jun 22, 2023
@ruthiel
Copy link

ruthiel commented Jun 23, 2023

"I'd prefer to use REF# as it offers a cleaner and simpler approach. However, I understand the argument that by using CHANGING, everyone will know that the variable will be modified within the method.

@bjoern-jueliger-sap
Copy link
Member

I think in general it is better to avoid mutating data in this manner altogether. In your example, is there is really a benefit to using change_by_ref instead of a method

methods add_to_parameter_name
  importing parameter_name type string
  returning value(result) type string.

and replacing

change_by_ref( ref #( variable ) ).

by

variable = add_to_parameter_name( variable ).

? Personally I find the latter much more readable - it is immediately clear to me that this line changes the content of variable based on its previous content.

@nununo
Copy link

nununo commented Jun 26, 2023

Fully agree with @bjoern-jueliger-sap. With the added bonus that method add_to_parameter_name can now be a pure function, making it much more straightforward to cover with unit tests.

@Yrfo
Copy link
Author

Yrfo commented Jun 26, 2023

@bjoern-jueliger-sap really nice way of doing that! I forgot about such variant. I completely agree that it's much cleaner in case of data types.

Would you do the same for object instances, when for example variable is instance of an object? Or just importing can be enough? I guess in the case of objects the argument about mutation is not so strong, they are expected to mutate at any time, isn't it?

@bjoern-jueliger-sap
Copy link
Member

@bjoern-jueliger-sap really nice way of doing that! I forgot about such variant. I completely agree that it's much cleaner in case of data types.

Would you do the same for object instances, when for example variable is instance of an object? Or just importing can be enough? I guess in the case of objects the argument about mutation is not so strong, they are expected to mutate at any time, isn't it?

For objects, the better solution is often to turn this into a method on the object. A function that takes an object instance as input and returns an object instance as output is isomorphic to a method on the object instance itself. (On the level of abstract type signatures, a method of an object type obj is just a function obj -> obj)

Concretely, if variable is an object, then that object type should define a method:

  methods add_to_parameter_name.

and we'd call it as

variable->add_to_parameter_name( ).

instead.

@bjoern-jueliger-sap
Copy link
Member

Closing this since there seems to be agreement that the guide's recommendation against CHANGING in most situations is appropriate as is.

@Yrfo
Copy link
Author

Yrfo commented Jul 4, 2023

@bjoern-jueliger-sap really nice way of doing that! I forgot about such variant. I completely agree that it's much cleaner in case of data types.
Would you do the same for object instances, when for example variable is instance of an object? Or just importing can be enough? I guess in the case of objects the argument about mutation is not so strong, they are expected to mutate at any time, isn't it?

For objects, the better solution is often to turn this into a method on the object. A function that takes an object instance as input and returns an object instance as output is isomorphic to a method on the object instance itself. (On the level of abstract type signatures, a method of an object type obj is just a function obj -> obj)

Concretely, if variable is an object, then that object type should define a method:

  methods add_to_parameter_name.

and we'd call it as

variable->add_to_parameter_name( ).

instead.

Ah, yes, it is true. Thank you guys for help! I'll try to remember that way of thinking 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clarity of Text The issue or pull request helps to improve the clarity of the text clean-abap
Projects
None yet
Development

No branches or pull requests

4 participants