[jira] Resolved: (COLLECTIONS-331) CollatingIterator NPE when Comparator is null

["Jochen Wiedmann (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/COLLECTIONS-331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jochen Wiedmann resolved COLLECTIONS-331.
-----------------------------------------

       Resolution: Fixed
    Fix Version/s:     (was: 4.0-beta-1)
                   3.2.2
         Assignee: Jochen Wiedmann

Agree with Hen's assessment that the use of Comparable.compareTo might likely cause a bunch of other problems.

I have verified, that this bug exists in every revision since CollatingIterator was verified. In other words, we don't need to care for upwards compatibility, there can't be any code floating around that depends on the use of "natural sort order".

Consequently I decided to

* throw an NPE in least() with a message describing the problem, if no Comparator is present. (This meets the current behaviour, so it is upwards compatible.)
* change the documentation of the default constructor to require a call to setComparator
* remove all references to "natural sort order" and replace them with the recommendation to use ComparableComparator, if required

> CollatingIterator NPE when Comparator is null
> ---------------------------------------------
>
>                 Key: COLLECTIONS-331
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-331
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Iterator
>    Affects Versions: 3.2
>            Reporter: Michael Krkoska
>            Assignee: Jochen Wiedmann
>            Priority: Minor
>             Fix For: 3.2.2
>
>         Attachments: COLLECTIONS-331.patch, CollatingIteratorTest.java
>
>   Original Estimate: 5m
>  Remaining Estimate: 5m
>
> Run the attached Test against commons-collections-3.2.1. While the JavaDoc for CollatingIterator claims 
> {{@param comp  the comparator to use to sort, or null to use natural sort order}}
> the test fails, because the comparator is null.
> Fixing this requires a change in {{org.apache.commons.collections.iterators.CollatingIterator.least()}}: 
> {code}
> if (comparator.compare(curObject,leastObject) < 0) { ...
> {code}
> could be replaced with
> {code}
> int c = comparator==null?((Comparable)curObject).compareTo(leastObject):comparator.compare(curObject,leastObject);
> if (c < 0) { ...
> {code}
> The workaround is to provide a comparator, of course.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira