Request for Review: CR#8001667, second attempt

Peter Levart peter.levart at
Fri Dec 7 09:09:18 UTC 2012

Hi Henry,

I don't know what the plans are about moving the static methods in 
Comparators to the Comparator interface when static interface methods 
are enabled, but if that is planned, there will be a clash between 
Comparator.reverseOrder() default method and static method with same 
name and signature.

Maybe rename static .reverseOrder() to .reverseNaturalOrder() (to 
long?), or rename the default method .reverseOrder() to .reverse();

Also, I like the natural-language-like fluent syntax when starting with 
Comparators.comparing(...).thenComparing(...).reverse[Order](), but the 
.thenComparing name is also overloaded with the variant for composing: 
.thenComparing(Comparator) which makes statements like:

Comparators.comparing(x -> x.size).thenComparing(x -> 
x.y).thenComparing((x, y) -> some expression)

a little confusing, because what you get is not a comparator that 
compares one value of "some expression" with some other value of the 
same expression, but a comparator that uses the value of expression to 
order two elements.

I think that using the same method name for two different purposes is a 
little confusing. Maybe .compose(Comparator) or just plain 
.then(Comparator) would be better here.

Regards, Peter

On 12/06/2012 07:57 AM, Henry Jen wrote:
> Hi,
> This update reflect changes based on feedbacks for last version, the
> changes are
> - rename reverse() to reverseOrder()
> - rename Comparator.compose to Comparator.thenComparing
> - add 4 Comparator.thenComparing methods take [Int|Long|Double]Function
> to enable fluent syntax like this example,
> people.sort(Comparators.comparing(People::getFirstName).thenComparing(People.getLastName))
> vs previously,
>     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] and specdiff[2].
> [1]
> [2]
> Cheers,
> Henry

More information about the core-libs-dev mailing list