Request for Review: CR#8001667: Comparators class and Comparator extension method
Stephen Colebourne
scolebourne at joda.org
Wed Nov 14 10:36:48 UTC 2012
On 14 November 2012 04:09, Henry Jen <henry.jen at oracle.com> wrote:
> This is a change set regarding Comparator already in lambda repo, it
> depends on the CR#8001634, particularly the Function SAMs.
>
> It implements proposed extension methods on Comparator
> (reverse and compose) as well as static combinator methods in
> Comparators for things like turning a T -> {Comparable,int,long,double}
> into a Comparator<T>.
>
> This allows things like:
>
> people.sort(Comparators.comparing(Person::getName))
> Comparator<Person> byLastFirst
> = Comparators.comparing(Person::getLastName)
> .compose(Comparators.comparing(Person::getFirstName))
>
> Please review and comment on the webrev[1].
Comparators declares a static serializable inner class
NaturalOrderComparator which is a singleton. Would it be worth
declaring this as an enum with a single instance? (as per Effective
Java version 2)
Comparators line 77 could do with a <p>
Comparators line 86: declaration about null that is covered in class
level Javadoc
Comparators line 92: who's -> whose
Comparators general: I'd like to see @code around types, such as
Comparable and Map.Entry
While "compose" feels like the right name for Comparators, it feels
like the wrong name for the equivalent method on Comparator (given the
example below). "andThen" or "then" would be fluent alternatives.
"composedWith" would be a more boring alternative.
byLastName.compose(byFirstName)
byLastName.andThen(byFirstName)
byLastName.then(byFirstName)
byLastName.composedWith(byFirstName)
Stephen
More information about the core-libs-dev
mailing list