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