Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Fixed
-
1.7.0
-
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)
}
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