Bikeshed opportunity: compose vs composeWith (original) (raw)

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 thenComparing(IntFunction 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 thenComparing(Comparator other) { return (Comparator & 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 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 byIdDesc, or Comparator BYBALANCE, 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 byLastThenByFirst = Comparators.by(Person::getLast).thenBy(Person::getFirst) or Comparator 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 comparingLastThenComparingFirst = Comparators.comparing(Person::getLast).thenComparing(Person::getFirst) or Comparator 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