Uploaded image for project: 'Metadata Aggregator'
  1. Metadata Aggregator
  2. MDA-221

Move from Guava collections to Java 9+ collections where possible

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.2
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None

      Description

      We use the Guava collections classes a fair bit in the MDA code, particularly on bean property setters. For example:

              if (ids == null || ids.isEmpty()) {
                  designatedEntities = Collections.emptyList();
              } else {
                  designatedEntities = ImmutableList.copyOf(Iterables.filter(ids, Predicates.notNull()));
              }
      

      With the Java 9 and Java 10 extensions to the Java collections classes, this can be replaced by the following:

              if (ids == null || ids.isEmpty()) {
                  designatedEntities = List.of();
              } else {
                  designatedEntities = List.copyOf(ids);
              }
      

      or

              if (ids == null) {
                  designatedEntities = List.of();
              } else {
                  designatedEntities = List.copyOf(ids);
              }
      

      This would change the MDA stage's API slightly, in that it would no longer accept nulls in the provided collection. I actually don't have a problem with that, and I've seen at least one presentation by the Java folks that indicates that they regard nulls in collections as one of their greatest mistakes; many of the newer collections APIs prohibit that.

      In any case, if we were going to make such an API change, pre-1.0 would be the time to do it. I think it's worth doing from first principles as a modernisation, but also to lower our area of contact with Guava.

      I think it's also worth noting that I've never been particularly happy about the fact that parameters of this kind are @Nullable in the MDA API, or at least they are some of the time. It's probably worth thinking about whether we need or want to preserve that part of the API, because if we can dump it then the above becomes just:

      designatedEntities = List.copyOf(ids);
      

      I think it's also worth pointing out that at least some of the bean property setters actually do the equivalent of that already, so it's not as if we're 100% consistent right now:

          public synchronized void setSchemaResources(@Nullable @NullableElements final List<Resource> resources) {
              ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
              ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
              
              schemaResources = ImmutableList.copyOf(Iterables.filter(resources, Predicates.notNull()));
          }
      

      I am pretty sure that the above will throw an NPE if presenting with a null parameter, despite the @Nullable decoration.

        Attachments

          Activity

            People

            Assignee:
            ian@iay.org.uk Ian Young
            Reporter:
            ian@iay.org.uk Ian Young
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - Not Specified
                Not Specified
                Logged:
                Time Spent - 28 minutes
                28m