I think it all boiled down to re-use an existing Comparator to compare
for another type.
What about we added this to Comparator<T> as a default method,
> default <S> Comparator<S> apply(Function<S, ? extends T> keyExtractor) {
> Objects.requireNonNull(keyExtractor);
> return (Comparator<S> & Serializable)
> (c1, c2) -> compare(keyExtractor.apply(c1),
> keyExtractor.apply(c2));
> }
Then the code you illustrated would be,
people.sort(CASE_INSENSITIVE_ORDER.apply(Person::getLastName)
.thenComparing(nullsLast.thenComparing(CASE_INSENSITIVE_ORDER)
.apply(Person::getEmailAddress)));
byKey and byValue is actually added based on the same thought that when
something is not a Comparable. With above, it can be replaced with
cmp.apply(Map.Entry<K,V>::getKey);
This is just inverse thinking of comparing, where the thoughts is mainly
for Comparable and primitive type. For those, comparing/thenComparing is
a more convenient and comprehensive expression.
Thoughts?
Cheers,
Henry
On 03/06/2013 03:06 PM, Michael Hixson wrote:
> On Wed, Mar 6, 2013 at 1:01 PM, Henry Jen <henry....@oracle.com> wrote:
>> On 03/06/2013 02:31 AM, Michael Hixson wrote:
>>
>>> 1. Why disable the following code even though it is (theoretically) safe?
>>>
>>> Comparator<CharSequence> a = comparing(CharSequence::length);
>>> Comparator<String> b = a.thenComparing(CASE_INSENSITIVE_ORDER);
>>>
>>> That code would compile if the signatures of the thenComparing methods
>>> had generics like <S extends T> instead of <T>. The example above is
>>> conjured up and ridiculous, but I think real code will have use for
>>> it. Is there any downside to narrowing the return type this way?
>>>
>>
>> I think that make sense, will need to do some experiment to make sure it
>> won't confuse type system when chaining all together, and I am not sure
>> how much burden this has on inference engine. I prefer simplicity.
>>
>> Theoretically, if all function take Comparator carefully allows super
>> type, which we have, isn't that enough? Your above example can be,
>>
>> Comparator<String> a = comparing<CharSequence::length);
>> a.thenComparing(CASE_INSENSITIVE_ORDER);
>>
>
> The idea is that I wanted to use both comparators. It's important
> that "a" remains Comparator<CharSequence> because I want to sort
> List<CharSequence> objects with it and use it to generate other
> CharSequence-subclass comparators in addition to "b".
>
> Also, min/max will need <S extends T> or else it will break Guava's
> Ordering class. The same thing happened a while back with
> comparator.reverse() (which was then renamed to reverseOrder). If the
> only reason for the rename was to unbreak Guava, then if you use <S
> extends T> you could change it back because the signatures will match.
>
> (Maybe the Guava devs have more insight about this signature? They
> went that route for most of Ordering's methods. Some of the same
> reasoning might apply here.)
>
>>
>>> 2. There's a thenComparing(Function), but no thenComparing(Function,
>>> Comparator). This seems like a big omission. Surely people will want
>>> secondary orderings on fields by something other than natural
>>> (null-unfriendly) ordering. Also, a Comparators.comparing(Function,
>>> Comparator) method would remove the need for all the Map-centric
>>> methods that are currently in the Comparators class. For instance:
>>> comparing(Map.Entry::getValue, naturalOrder()).
>>>
>>
>> Note that Function form must return a Comparable, which implies an order
>> already. Combine with Comparator.reverseOrder() method, that would cover
>> the ground.
>>
>> If the Comparable is not a fit, probably write a Comparator in lambda is
>> needed anyway?
>>
>>> 3. There is no support for sorting of nullable values or fields.
>>> Sorting of nullable values directly perhaps should not be encouraged,
>>> but sorting values by nullable fields within a chained sort is
>>> completely reasonable. Please add some support for this, either as
>>> chain methods on Comparator, or as factory methods on Comparators.
>>>
>>
>> Not sure what do you propose to be added. NULL is a controversial topic,
>> only application know what it means. Therefore, avoid try to interpret
>> null is probably a better approach. Write a Comparator if needed.
>>
>
> Regarding comparing(Function,comparator) and nulls - I'm possibly just
> repeating old arguments but I'll give it another shot.
>
> There are use cases for these all over the code base I ported to Java
> 8. I'll repost the example from my first email since I think that may
> answer your question about nulls:
>
> // Sort a list of people by:
> // 1) Last name, ignoring case
> // 2) Email address, with no email (null) last, ignoring case
> people.sort(
> comparing(Person::getLastName, CASE_INSENSITIVE_ORDER)
> .thenComparing(
> Person::getEmailAddress,
> nullsLast().thenComparing(CASE_INSENSITIVE_ORDER)));
>
> The Comparators class itself presents four use cases for
> comparing(Function,Comparator). I don't think they're especially good
> cases, but: naturalOrderKeys, naturalOrderValues, byKey, byValue could
> all be done instead as comparing(Map.Entry::get{Key,Value},c). It is
> odd to me that four specialized versions of this are being offered (I
> can't recall the last time I wanted to sort map entries) but the more
> primitive operation behind them is not.
>
> -Michael
>
