Uploaded image for project: 'mdsal'
  1. mdsal
  2. MDSAL-374

Keyed list entry builders' copy constructor checks for null key

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Medium
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 3.0.1
    • Component/s: Binding V1 codegen
    • Labels:
      None

      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.

        Attachments

          Issue Links

          # Subject Branch Project Status CR V

            Activity

              People

              • Assignee:
                rovarga Robert Varga
                Reporter:
                rovarga Robert Varga
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: