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.

Sign up for GitHub

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 FINAL declarations for immutable variables #299

Closed
wants to merge 4 commits into from
Closed

Use FINAL declarations for immutable variables #299

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6f00e5
Add rule for use of FINAL keyword
ConjuringCoffee Feb 20, 2023
3aaa340
Use FINAL in all applicable examples
ConjuringCoffee Feb 20, 2023
ecab23d
Add missing links to new rule
ConjuringCoffee Feb 21, 2023
55f2531
Use suggestion to address FINAL subleties
ConjuringCoffee Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 30 additions & 22 deletions clean-abap/CleanABAP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ The [Cheat Sheet](cheat-sheet/CheatSheet.md) is a print-optimized version.
- [Variables](#variables)
- [Prefer inline to up-front declarations](#prefer-inline-to-up-front-declarations)
- [Don't declare inline in optional branches](#dont-declare-inline-in-optional-branches)
- [Use FINAL declarations for immutable variables](#use-final-declarations-for-immutable-variables)
- [Do not chain up-front declarations](#do-not-chain-up-front-declarations)
- [Prefer REF TO to FIELD-SYMBOL](#prefer-ref-to-to-field-symbol)
- [Tables](#tables)
Expand Down Expand Up @@ -679,7 +680,7 @@ these objects should do little more than call a corresponding class that provide

```ABAP
FUNCTION check_business_partner [...].
DATA(validator) = NEW /clean/biz_partner_validator( ).
FINAL(validator) = NEW /clean/biz_partner_validator( ).
result = validator->validate( business_partners ).
ENDFUNCTION.
```
Expand Down Expand Up @@ -911,8 +912,8 @@ that declaring variables inline at first occurrence will look more natural

```ABAP
METHOD do_something.
DATA(name) = 'something'.
DATA(reader) = /clean/reader=>get_instance_for( name ).
FINAL(name) = 'something'.
FINAL(reader) = /clean/reader=>get_instance_for( name ).
result = reader->read_it( ).
ENDMETHOD.
```
Expand Down Expand Up @@ -962,6 +963,13 @@ ENDIF.

> Read more in _Chapter 5: Formatting: Vertical Distance: Variable Declarations_ of [Robert C. Martin's _Clean Code_].

### Use FINAL declarations for immutable variables

> [Clean ABAP](#clean-abap) > [Content](#content) > [Variables](#variables) > [This section](#use-final-declarations-for-immutable-variables)

If a variable is never reassigned use `FINAL` to declare it.
The reader can then be sure that the content of the variable is only ever changed at the source position where it is declared.

### Do not chain up-front declarations

> [Clean ABAP](#clean-abap) > [Content](#content) > [Variables](#variables) > [This section](#do-not-chain-up-front-declarations)
Expand Down Expand Up @@ -1001,7 +1009,7 @@ DATA:
> for these cases may worsen performance.

```ABAP
LOOP AT components REFERENCE INTO DATA(component).
LOOP AT components REFERENCE INTO FINAL(component).
```

instead of the equivalent
Expand Down Expand Up @@ -1506,8 +1514,8 @@ It's nearly always a good idea to extract complex conditions to methods of their
IF is_provided( example ).

METHOD is_provided.
DATA(is_filled) = xsdbool( example IS NOT INITIAL ).
DATA(is_working) = xsdbool( applies( example ) = abap_true OR
FINAL(is_filled) = xsdbool( example IS NOT INITIAL ).
FINAL(is_working) = xsdbool( applies( example ) = abap_true OR
fits( example ) = abap_true ).
result = xsdbool( is_filled = abap_true AND
is_working = abap_true ).
Expand Down Expand Up @@ -1699,7 +1707,7 @@ CLASS /clean/string_utils DEFINITION [...].
ENDCLASS.

METHOD retrieve.
DATA(trimmed_name) = /clean/string_utils=>trim( name ).
FINAL(trimmed_name) = /clean/string_utils=>trim( name ).
result = read( trimmed_name ).
ENDMETHOD.
```
Expand Down Expand Up @@ -2581,7 +2589,7 @@ METHODS update_references
bo_nodes TYPE root_nodes.

METHOD update_references.
LOOP AT bo_nodes REFERENCE INTO DATA(bo_node).
LOOP AT bo_nodes REFERENCE INTO FINAL(bo_node).
bo_node->reference = new_reference.
ENDLOOP.
ENDMETHOD.
Expand Down Expand Up @@ -2864,7 +2872,7 @@ instead of confusing mixtures of low level (`trim`, `to_upper`, ...) and high le
" anti-pattern
METHOD create_and_publish.
post = NEW blog_post( ).
DATA(user_name) = trim( to_upper( sy-uname ) ).
FINAL(user_name) = trim( to_upper( sy-uname ) ).
post->set_author( user_name ).
post->publish( ).
ENDMETHOD.
Expand All @@ -2882,8 +2890,8 @@ Methods should have less than 20 statements, optimal around 3 to 5 statements.

```ABAP
METHOD read_and_parse_version_filters.
DATA(active_model_version) = read_random_version_under( model_guid ).
DATA(filter_json) = read_model_version_filters( active_model_version-guid ).
FINAL(active_model_version) = read_random_version_under( model_guid ).
FINAL(filter_json) = read_model_version_filters( active_model_version-guid ).
result = parse_model_version_filters( filter_json ).
ENDMETHOD.
```
Expand Down Expand Up @@ -2964,7 +2972,7 @@ METHOD do_something.
IF input IS INITIAL.
RAISE EXCEPTION cx_sy_illegal_argument( ).
ENDIF.
DATA(massive_object) = build_expensive_object_from( input ).
FINAL(massive_object) = build_expensive_object_from( input ).
result = massive_object->do_some_fancy_calculation( ).
ENDMETHOD.
```
Expand All @@ -2974,7 +2982,7 @@ Later validations are harder to spot and understand and may have already wasted
```ABAP
" anti-pattern
METHOD do_something.
DATA(massive_object) = build_expensive_object_from( input ).
FINAL(massive_object) = build_expensive_object_from( input ).
IF massive_object IS NOT BOUND. " happens if input is initial
RAISE EXCEPTION cx_sy_illegal_argument( ).
ENDIF.
Expand Down Expand Up @@ -3441,7 +3449,7 @@ METHODS generate RAISING cx_generation_failure.
METHOD generate.
TRY.
generator->generate( ).
CATCH cx_amdp_generation_failure INTO DATA(exception).
CATCH cx_amdp_generation_failure INTO FINAL(exception).
RAISE EXCEPTION NEW cx_generation_failure( previous = exception ).
ENDTRY.
ENDMETHOD.
Expand Down Expand Up @@ -4589,7 +4597,7 @@ CLASS /dirty/default_custom_reader IMPLEMENTATION.
ENDCLASS.

METHOD test_something.
DATA(customizing_reader) = NEW /dirty/customizing_reader( ).
FINAL(customizing_reader) = NEW /dirty/customizing_reader( ).
customizing_reader->customizing = sub_claim_customizing.
ENDMETHOD.
```
Expand Down Expand Up @@ -4641,7 +4649,7 @@ CLASS /clean/class_under_test DEFINITION LOCAL FRIENDS unit_tests.

CLASS unit_tests IMPLEMENTATION.
METHOD setup.
DATA(mock) = cl_abap_testdouble=>create( '/clean/some_mock' ).
FINAL(mock) = cl_abap_testdouble=>create( '/clean/some_mock' ).
" /clean/class_under_test is CREATE PRIVATE
" so this only works because of the LOCAL FRIENDS
cut = NEW /clean/class_under_test( mock ).
Expand Down Expand Up @@ -4820,7 +4828,7 @@ Make sure that the "when" section of your test method contains exactly one call
```ABAP
METHOD rejects_invalid_input.
" when
DATA(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
FINAL(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
" then
cl_abap_unit_assert=>assert_false( is_valid ).
ENDMETHOD.
Expand Down Expand Up @@ -4910,7 +4918,7 @@ Assert only exactly what the test method is about, and this with a small number
```ABAP
METHOD rejects_invalid_input.
" when
DATA(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
FINAL(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
" then
cl_abap_unit_assert=>assert_false( is_valid ).
ENDMETHOD.
Expand All @@ -4926,7 +4934,7 @@ obscuring the one important, distinguishing assertion among them.
" anti-pattern
METHOD rejects_invalid_input.
" when
DATA(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
FINAL(is_valid) = cut->is_valid_input( 'SOME_RANDOM_ENTRY' ).
" then
cl_abap_unit_assert=>assert_false( is_valid ).
cl_abap_unit_assert=>assert_not_initial( log->get_messages( ) ).
Expand Down Expand Up @@ -5020,7 +5028,7 @@ METHODS reads_entry FOR TESTING RAISING /clean/some_exception.

METHOD reads_entry.
"when
DATA(entry) = cut->read_something( ).
FINAL(entry) = cut->read_something( ).
"then
cl_abap_unit_assert=>assert_not_initial( entry ).
ENDMETHOD.
Expand All @@ -5032,8 +5040,8 @@ Your test code remains focused on the happy path and is therefore much easier to
" anti-pattern
METHOD reads_entry.
TRY.
DATA(entry) = cut->read_something( ).
CATCH /clean/some_exception INTO DATA(unexpected_exception).
FINAL(entry) = cut->read_something( ).
CATCH /clean/some_exception INTO FINAL(unexpected_exception).
cl_abap_unit_assert=>fail( unexpected_exception->get_text( ) ).
ENDTRY.
cl_abap_unit_assert=>assert_not_initial( entry ).
Expand Down