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

Keyed list entry builders' copy constructor checks for null key

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Done
    • Icon: Medium Medium
    • 3.0.1
    • None
    • Binding codegen
    • None

      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.

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

              Created:
              Updated:
              Resolved: