[MDSAL-374] Keyed list entry builders' copy constructor checks for null key Created: 10/Oct/18  Updated: 12/Oct/18  Resolved: 12/Oct/18

Status: Resolved
Project: mdsal
Component/s: Binding codegen
Affects Version/s: None
Fix Version/s: 3.0.1

Type: Improvement Priority: Medium
Reporter: Robert Varga Assignee: Robert Varga
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Blocks
blocks MDSAL-375 Do not store duplicate keyed list ent... Confirmed
blocks MDSAL-376 Keyed list entry builder should cross... Confirmed

 Description   

When we generate a Builder and its corresponding internal implementation we are performing the following check:

            if (base.key() == null) {
                this.key = new LstKey(
                    base.getKey()
                );
                this._key = base.getKey();
            } else {
                this.key = base.key();
                this._key = key.getKey();
            }
            this.augmentation = ImmutableMap.copyOf(base.augmentation);

Note that getKey() here refers to the leaf which is a part of the key. This is something we are reusing between builder and implementation, hence they look essentially the same.

Ever since we correctly override key() from Identifiable, Eclipse is warning us that the invocation in Builder's copy constructor is superfluous: key() is required to return non-null, hence there is simply no point in checking for it being null and having that boiler plate present.

key() method here is also special (a bit), as it should not be a straight-up getter, but it should instantiate the key lazily on-demand. That obviously means that key constituent fields need to be checked for nullness during implementation instantiation – note that IllegalStateException can be thrown from Builder.build().

These two points should addressed separately, with the builder copy constructor piece being addressed first.



 Comments   
Comment by Robert Varga [ 10/Oct/18 ]

The second part of this issue is split off in MDSAL-375. An additional improvement to builders lifecycle is tracked in MDSAL-376. This issue should provide baseline refactoring of AbstractBuilderTemplate so that the two follow-ups work on independently.

Generated at Wed Feb 07 20:09:39 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.