SAP / styleguides

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

Use FINAL declarations for immutable variables #299

Closed ConjuringCoffee closed 1 year ago

ConjuringCoffee commented 1 year ago

Fixes #285.

I've added the new rule and changed all examples in which it was obvious that a variable is never reassigned.

ConjuringCoffee commented 1 year ago

Now here's a problem: FINAL doesn't just prevent changes in unexpected locations in the source code, it also affects the debugger. You can't change the variable in the debugger anymore.

I find this to be a major downside of the new keyword. Does anyone know if there's a workaround or a setting for the debugger? If not, then I'm personally quite conflicted about the new rule. I've converted the pull request to a draft until further clarification took place.

ChristophStoeckSAP commented 1 year ago

One comment form the ABAP Debugger perspective: There is no way to change FINAL variables in the ABAP debugger. This is the same as with constants.

bjoern-jueliger-sap commented 1 year ago

The issue with final is a bit more subtle than "use it if a variable is never reassigned", and the fact that final variables cannot be changed in the debugger (and that this is surprising to at least some users of the feature) hints at the underlying mismatch in concepts of immutability:

While it may be tempting to think about final as a hint to the compiler to emit syntax errors when you try to change that variable again, final variables in fact have different runtime properties than normal variables: The notion of immutability is a runtime property in ABAP, not a compile-time property!

The following code is syntax-error free, but produces a short dump at runtime:

final(var) = 1.
assign var to field-symbol(<var>).
<var> = 11.

Changing the data declaration to use data instead of final avoids the short dump.

This is probably not what you expect if you are used to thinking about immutability as a compile-time property of bindings (like const variables in Javascript or non-mut variables in Rust), even though the documentation of final says it: "In all other positions, any write access leads either to a syntax error or the uncatchable exception MOVE_TO_LIT_NOTALLOWED_NODATA." It's important to realize that "write access" refers to the data object created by the final declaration, not to write access to the variable name (the binding).

In the example above it is easy to understand why the dump happens once you realize final sets a run-time property on the data object that underlies var, but if the dump involves a call stack and references, this can get difficult to analyze. For instance, it is not a syntax error to use a reference to a final variable as the actual parameter of another method:

final(var) = 1.
not_my_method( ref #( var ) ).

If not_my_method then at any point attempts to change the content of the reference, we get a short dump - even though var is never "reassigned" in the part of the program that is under our control.

So while I think a recommendation in the spirit of "Use final whenever appropriate" should be made, I think any such recommendation needs to have some details about what "appropriate" means: Especially when references are involved, the programmer needs to think carefully about whether or not the write-protection that final generates at runtime is intended or not.

ConjuringCoffee commented 1 year ago

Great comment. I am closing this pull request and pointed the issue to it.