Uploaded image for project: 'Commons BeanUtils'
  1. Commons BeanUtils
  2. BEANUTILS-267

BeanComparator(String, Comparator) should check the comparator for null and default to ComparableComparator.getInstance()

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7.0
    • Fix Version/s: 1.8.0
    • Component/s: Bean-Collections
    • Labels:
      None

      Description

      The way the BeanComparator(String, Comparator) constructor is implemented is inconvenient. I've got code that passes in a comparator. This comparator may be null. I assumed that the 2-args constructor would sanely ignore a null comparator argument and use a default like ReverseComparator does in commons-collections, but alas, no. I have to do the null check before I pass it in

      For instance, here's the constructor for ReverseComparator, which takes a Comparator argument...

      public ReverseComparator(Comparator comparator) {
      if(comparator != null)

      { this.comparator = comparator; } else { this.comparator = ComparableComparator.getInstance(); }
      }

      The null check and provided default is convenient and reasonable.

      Here's the current constructor for BeanComparator that can only end in a NullPointerException if provided null comparator....

      public BeanComparator( String property, Comparator comparator ) { setProperty( property ); this.comparator = comparator; }

      Why not?....

      public BeanComparator( String property, Comparator comparator ) {
      setProperty( property );
      if(comparator != null) { this.comparator = comparator; }

      else

      { this.comparator = ComparableComparator.getInstance(); }

      }

      The fact that BeanComparator allows itself to be put in a bad state by storing a null comparator which it later tries to use with no null check, guaranteeing a NullPointerException, probably should be considered a bug. However, since it works just fine when provided a non-null comparator, I consider this more of an "Improvement" opportunity than a bug, thus the reported Issue Type. Hopefully this can be applied to the next release.

      Jake

        Attachments

          Activity

            People

            • Assignee:
              niallp Niall Pemberton
              Reporter:
              hoju Jacob Kjome
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: