Bikeshed opportunity: compose vs composeWith
Brian Goetz
brian.goetz at oracle.com
Thu Nov 29 07:57:19 PST 2012
> No problem. Yes, “by” here would take Function. Looks like your
> variable naming is pretty much like our naming, which is an obvious
> choice for variable names in this case IMO. I would be fine with
> thenComparing taking Comparator.
I'm OK with thenComparing taking comparator and having convenience
methods to take a Function too:
default Comparator<T> thenComparing(IntFunction<T> f) {
return thenComparing(comparing(f));
}
I'm with Kevin on "thenComparing" over "by".
> If we can get agreement on the name, I’d love to then jump in the
> discussion about Serializability of chained/composed
> Comparators/Functions. I might be missing something obvious, but
> without extending Serializable on Comparator and Function, there is
> going to have to be some interesting instanceof checking inside
> by/comparing/thenBy/thenComparing/reverse/etc.
For thenComparing(Comparator), we can follow the convention set in the
JDK of making combinators serializable on the off chance that their
arguments are (see the existing implementation of
Collections.reverseOrder()). With the long-battled opt-in syntax, this
becomes pretty easy:
default Comparator<T> thenComparing(Comparator<T> other) {
return (Comparator<T> & Serializable) (a, b) -> {
int cmp = compare(a,b);
return cmp == 0 ? other.compare(a,b) : cmp;
}
}
If the underlying comparators are serializable, then so is the composed
one. It is a nasty compromise but seems reasonable for combinators like
Predicate.and, etc?
> How would we make this Serializable?
>
> TreeSet<Person> peopleByLastNameFirstNameAndAgeDesc = new
> TreeSet(by(Person::getLast).thenBy(Person::getAge).thenComparing(by(Person::getAge).reverse()));
>
> I’m sure some folks will want to say “Edge case! Foul! No one uses
> serialization!”. That’s of course is the easy way out. ;-) But maybe
> there is actually an easy way out I haven’t thought of or am not aware
> of because of some discussion thread I may have unfortunately missed.
>
> *From:*Kevin Bourrillion [mailto:kevinb at google.com]
> *Sent:* Thursday, November 29, 2012 2:45 AM
> *To:* Raab, Donald [Tech]
> *Cc:* Brian Goetz; Sam Pullara; lambda-libs-spec-experts at openjdk.java.net
> *Subject:* Re: Bikeshed opportunity: compose vs composeWith
>
> As soon as I hit send I realized I was thinking of the *by() methods as
> taking Comparators as their parameters, which is wrong. Perhaps
> responding on this thread at 2:45 am was a suboptimal idea. Sorry!
>
> On Thu, Nov 29, 2012 at 2:43 AM, Kevin Bourrillion <kevinb at google.com
> <mailto:kevinb at google.com>> wrote:
>
> Note that a very natural way that users are naming their comparator
> instances is to start with "by" : Comparator<User> byIdDesc, or
> Comparator<Account> BY_BALANCE, etc. I know that this is "common" in
> Google code (I could get more specific if necessary).
>
> These, of course, would become dreadful names overnight if the method
> names we use themselves end in "by".
>
> That's just one more small reason why my preference is still for
> comparing/thenComparing.
>
> On Tue, Nov 27, 2012 at 5:11 PM, Raab, Donald <Donald.Raab at gs.com
> <mailto:Donald.Raab at gs.com>> wrote:
>
> Here's my last try at "by". It's concise and readable in my opinion.
> For any of us collections framework developers who already have
> sortBy, minBy and maxBy on our protocols it also offers nice symmetry
> when used in the fluent form with static imports.
>
> people.sort(by(Person::getLast).thenBy(Person::getFirst))
> people.min(by(Person::getLast).thenBy(Person::getFirst))
> people.max(by(Person::getLast).thenBy(Person::getFirst))
>
> Comparator<Person> byLastThenByFirst =
> Comparators.by(Person::getLast).thenBy(Person::getFirst)
>
> or
>
> Comparator<Person> byLastThenByFirst =
> by(Person::getLast).thenBy(Person::getFirst)
>
> Vs.
>
> people.sort(comparing(Person::getLast).thenComparing(Person::getFirst))
> people.min(comparing(Person::getLast).thenComparing(Person::getFirst))
> people.max(comparing(Person::getLast).thenComparing(Person::getFirst))
>
> Comparator<Person> comparingLastThenComparingFirst =
> Comparators.comparing(Person::getLast).thenComparing(Person::getFirst)
>
> or
>
> Comparator<Person> comparingLastThenComparingFirst =
> comparing(Person::getLast).thenComparing (Person::getFirst)
>
>
> Both read ok, so whatever the consensus is.
>
>
>> -----Original Message-----
>> From: Brian Goetz [mailto:brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>]
>> Sent: Tuesday, November 27, 2012 11:04 AM
>> To: Kevin Bourrillion
>> Cc: Raab, Donald [Tech]; Sam Pullara; lambda-libs-spec-
>>experts at openjdk.java.net <mailto:experts at openjdk.java.net>
>> Subject: Re: Bikeshed opportunity: compose vs composeWith
>>
>> I like "thenComparing". Done?
>>
>> Separately, would "reverseOrder" be better / more consistent for the
>> extension method on Comparator than the current "reverse"? (There's
>> already a Collections.reverseOrder static method.)
>>
>> On 11/27/2012 10:35 AM, Kevin Bourrillion wrote:
>> > I think it's very helpful for these two names to have parallel
>> structure:
>> >
>> > comparingBy / thenBy
>> >
>> > or
>> >
>> > comparing / thenComparing
>> >
>> >
>> >
>> >
>> > On Mon, Nov 26, 2012 at 2:48 PM, Raab, Donald <Donald.Raab at gs.com <mailto:Donald.Raab at gs.com>
>> > <mailto:Donald.Raab at gs.com <mailto:Donald.Raab at gs.com>>> wrote:
>> >
>> > What about this?
>> >
>> > people.sort(comparing(Person::getLast).thenBy(Person::getFirst))
>> >
>> > > -----Original Message-----
>> > > From: Raab, Donald [Tech]
>> > > Sent: Monday, November 26, 2012 5:45 PM
>> > > To: 'Brian Goetz'; Sam Pullara
>> > > Cc:lambda-libs-spec-experts at openjdk.java.net
> <mailto:lambda-libs-spec-experts at openjdk.java.net>
>> > <mailto:lambda-libs-spec-experts at openjdk.java.net
> <mailto:lambda-libs-spec-experts at openjdk.java.net>>
>> > > Subject: RE: Bikeshed opportunity: compose vs composeWith
>> > >
>> > > Just out of curiosity,I opened up Microsoft Excel and looked
>> how they
>> > > word multi-level sorts.
>> > >
>> > > Sort By -> Column A
>> > > Then By -> Column B
>> > > Then By -> Column C
>> > > Etc.
>> > >
>> > > So "then" in the wording definitely looks good. Not sure if
>> there is
>> > > anything better than thenCompare.
>> > >
>> > > This would be cool to read, but is a different animal.
>> > >
>> > > people.sortBy(Person::getLast).thenBy(Person::getFirst)
>> > >
>> > > We use the word "chain" on our Comparators static utility
>> class in GS
>> > > Collections with varargs. But since you are adding this
>> method to
>> > > Comparator, not sure if chain or chainWith would be a good
>> name.
>> > >
>> > > > -----Original Message-----
>> > > > From:lambda-libs-spec-experts-bounces at openjdk.java.net
> <mailto:lambda-libs-spec-experts-bounces at openjdk.java.net>
>> > <mailto:lambda-libs-spec-experts-bounces at openjdk.java.net
> <mailto:lambda-libs-spec-experts-bounces at openjdk.java.net>>
>> > > > [mailto:lambda- <mailto:lambda-> <mailto:lambda- <mailto:lambda->>
>> >libs-spec-experts-bounces at openjdk.java.net
> <mailto:libs-spec-experts-bounces at openjdk.java.net>
>> > <mailto:libs-spec-experts-bounces at openjdk.java.net
> <mailto:libs-spec-experts-bounces at openjdk.java.net>>] On Behalf
>> > > > Of Brian Goetz
>> > > > Sent: Monday, November 26, 2012 3:07 PM
>> > > > To: Sam Pullara
>> > > > Cc:lambda-libs-spec-experts at openjdk.java.net
> <mailto:lambda-libs-spec-experts at openjdk.java.net>
>> > <mailto:lambda-libs-spec-experts at openjdk.java.net
> <mailto:lambda-libs-spec-experts at openjdk.java.net>>
>> > > > Subject: Re: Bikeshed opportunity: compose vs composeWith
>> > > >
>> > > > I like the "then" convention to indicate sequencing. In
>> context:
>> > > >
>> > > > people.sort(comparing(Person::getLast)
>> > > > .thenCompare(comparing(Person::getFirst)))
>> > > >
>> > > >
>> > > >
>> > > > On 11/26/2012 3:04 PM, Sam Pullara wrote:
>> > > > > How about something that sounds more comparator specific:
>> > > > >
>> > > > > comparator1.thenCompare(comparator2)
>> > > > >
>> > > > > Sam
>> > > > >
>> > > > > On Nov 26, 2012, at 11:57 AM, Kevin Bourrillion
>> > <kevinb at google.com <mailto:kevinb at google.com> <mailto:kevinb at google.com
> <mailto:kevinb at google.com>>
>> > > > > <mailto:kevinb at google.com <mailto:kevinb at google.com> <mailto:kevinb at google.com
> <mailto:kevinb at google.com>>>>
>> wrote:
>> > > > >
>> > > > >> So... comparator1.compound(comparator2)?
>> > > > >>
>> > > > >>
>> > > > >> On Mon, Nov 26, 2012 at 11:10 AM, Brian Goetz
>> > > > <brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>
> <mailto:brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>>
>> > > > >> <mailto:brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>
>> > <mailto:brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>>>> wrote:
>> > > > >>
>> > > > >> However, this is the first time I'm noticing that
>> you're
>> > > > using
>> > > > >> the name
>> > > > >> compose() not only for function composition, but
>> > also for
>> > > > >> forming a
>> > > > >> compound comparator. Has it been suggested that
>> we not
>> > > > reuse the
>> > > > >> compose() name to mean this other thing? Note
>> that
>> > there
>> > > > does
>> > > > >> exist a
>> > > > >> compose operation for Comparators, but it's
>> (Function,
>> > > > >> Comparator) ->
>> > > > >> Comparator (Guava puts it in the other order and
>> > calls it
>> > > > >> "onResultOf",
>> > > > >> which I'm not recommending).
>> > > > >>
>> > > > >>
>> > > > >> It has not been suggested until now. I am fine
>> calling this
>> > > > >> something that does not contain the string "compose".
>> > The key
>> > > > >> concept is "I have two comparators, and I want to
>> build a
>> > > > >> dictionary-order comparator for (O1, O2)."
>> > > > >>
>> > > > >> I am fine with .compose() for functions.
>> > > > >>
>> > > > >> I think .compose(other) is too cryptic for
>> comparators. I
>> > > think
>> > > > >> .composeWith() is better; I can imagine there are
>> other
>> > things
>> > > > >> that are also better. Now taking suggestions.
>> (Though
>> > > > onResultOf
>> > > > >> does not seem better.)
>> > > > >>
>> > > > >>
>> > > > >>
>> > > > >>
>> > > > >> --
>> > > > >> Kevin Bourrillion | Java Librarian | Google, Inc.
>> > > > >> |kevinb at google.com <mailto:kevinb at google.com> <mailto:kevinb at google.com
> <mailto:kevinb at google.com>>
>> > <mailto:kevinb at google.com <mailto:kevinb at google.com> <mailto:kevinb at google.com
> <mailto:kevinb at google.com>>>
>> > > > >>
>> > > > >
>> >
>> >
>> >
>> >
>> > --
>> > Kevin Bourrillion | Java Librarian | Google, Inc. |kevinb at google.com <mailto:kevinb at google.com>
>> > <mailto:kevinb at google.com <mailto:kevinb at google.com>>
>> >
>
>
>
> --
>
> Kevin Bourrillion | Java Librarian | Google, Inc. |kevinb at google.com
> <mailto:kevinb at google.com>
>
>
>
> --
>
> Kevin Bourrillion | Java Librarian | Google, Inc. |kevinb at google.com
> <mailto:kevinb at google.com>
>
More information about the lambda-libs-spec-observers
mailing list