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

Prefer CORRESPONDING # to MOVE-CORRESPONDING #521

Open
lucasborin opened this issue Nov 5, 2021 · 6 comments
Open

Prefer CORRESPONDING # to MOVE-CORRESPONDING #521

lucasborin opened this issue Nov 5, 2021 · 6 comments
Labels
new check New check

Comments

@lucasborin
Copy link
Member

instead of:

DATA reference_object TYPE tadir.
DATA new_object TYPE tadir.

MOVE-CORRESPONDING reference_object TO new_object.

use:

DATA reference_object TYPE tadir.
DATA new_object TYPE tadir.

new_object = CORRESPONDING #( reference_object ).
@lucasborin lucasborin added the new check New check label Nov 5, 2021
@bjoern-jueliger-sap
Copy link
Member

There seems to be no section in the style guide that actually recommends this. If we promise "adherence to Clean ABAP" in our documentation, we shouldn't invent new checks out of thin air.

@pokrakam
Copy link
Contributor

Doesn't this fall under Prefer functional to procedural language constructs?
Although technically not "functional" per se, the examples refer to constructor operators NEW and VALUE, so I would consider corresponding included.

@fabianlupa
Copy link

Note that target and source structure need to be of the same type for the constructor expression to have the same result as the statement, a check would have to account for this.

@bjoern-jueliger-sap
Copy link
Member

From the documentation of CORRESPONDING:

An assignment of structures

struct2 = CORRESPONDING #( struct1 ).

without the addition BASE is not the same as an assignment

MOVE-CORRESPONDING struct1 TO struct2.

In MOVE-CORRESPONDING, all not identically named components in struct2 keep their value. When the result of the constructor expression is assigned, however, they are assigned the value from there, which is initial for ignored components. This behavior can be overridden using the addition BASE.

This means that the "functional" equivalent form of

MOVE-CORRESPONDING source TO target.

would be

target = corresponding #( base ( target ) source ).

I suspect CORRESPONDING is explicitly not part of the examples in the guide because this form - in contrast to e.g. NEW vs. CREATE OBJECT - isn't obviously easier to read then the MOVE-CORRESPONDING form. At least I personally always stop and need to think a bit about what base does when I encounter it.

@pokrakam
Copy link
Contributor

Valid point, but I think off topic regarding this issue. The check is there to suggest that we should avoid MOVE-CORRESPONDING, and I feel this is the right thing both in terms of ABAP and in line with the Style Guide.

Seeing variants of CORRESPONDING #( in code makes it explicit. MOVE-CORRESPONDING is not clear whether leaving missing component values intact was intended or accidental. In my experience most people are not aware of it as I've had to deal with bugs stemming from this on several occasions.

I also think that leaving it from the style guide example was more coincidental, there are many constructor operators, and NEW and VALUE is sufficient to indicate we should also use REF, EXACT etc., the examples are long enough as is.

@bjoern-jueliger-sap
Copy link
Member

The argument that CORRESPONDING at least is explicit about whether or not the base is kept sounds convincing to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new check New check
Projects
None yet
Development

No branches or pull requests

4 participants