106

I've been looking at the difference between Collections.sort and list.sort, specifically regarding using the Comparator static methods and whether param types are required in the lambda expressions. Before we start, I know I could use method references, e.g. Song::getTitle to overcome my problems, but my query here is not so much something I want to fix but something I want an answer to, i.e. why is the Java compiler handling it in this way.

These are my finding. Suppose we have an ArrayList of type Song, with some songs added, there are 3 standard get methods:

    ArrayList<Song> playlist1 = new ArrayList<Song>();

    //add some new Song objects
    playlist.addSong( new Song("Only Girl (In The World)", 235, "Rhianna") );
    playlist.addSong( new Song("Thinking of Me", 206, "Olly Murs") );
    playlist.addSong( new Song("Raise Your Glass", 202,"P!nk") );

Here is a call to both types of sort method that works, no problem:

Collections.sort(playlist1, 
            Comparator.comparing(p1 -> p1.getTitle()));

playlist1.sort(
            Comparator.comparing(p1 -> p1.getTitle()));

As soon as I start to chain thenComparing, the following happens:

Collections.sort(playlist1,
            Comparator.comparing(p1 -> p1.getTitle())
            .thenComparing(p1 -> p1.getDuration())
            .thenComparing(p1 -> p1.getArtist())
            );
    
playlist1.sort(
        Comparator.comparing(p1 -> p1.getTitle())
        .thenComparing(p1 -> p1.getDuration())
        .thenComparing(p1 -> p1.getArtist())
        );

i.e. syntax errors because it does not know the type of p1 anymore. So to fix this I add the type Song to the first parameter (of comparing):

Collections.sort(playlist1,
            Comparator.comparing((Song p1) -> p1.getTitle())
            .thenComparing(p1 -> p1.getDuration())
            .thenComparing(p1 -> p1.getArtist())
            );
    
playlist1.sort(
        Comparator.comparing((Song p1) -> p1.getTitle())
        .thenComparing(p1 -> p1.getDuration())
        .thenComparing(p1 -> p1.getArtist())
        );

Now here comes the CONFUSING part. For playlist1.sort, i.e. the List, this solve all compilation errors, for both the following thenComparing calls. However, for Collections.sort, it solves it for the first one, but not the last one. I tested added several extra calls to thenComparing and it always shows an error for the last one, unless I put (Song p1) for the parameter.

Now I went on to test this further with creating a TreeSet and with using Objects.compare:

int x = Objects.compare(t1, t2, 
                Comparator.comparing((Song p1) -> p1.getTitle())
                .thenComparing(p1 -> p1.getDuration())
                .thenComparing(p1 -> p1.getArtist())
                );
    
    
    Set<Song> set = new TreeSet<Song>(
            Comparator.comparing((Song p1) -> p1.getTitle())
            .thenComparing(p1 -> p1.getDuration())
            .thenComparing(p1 -> p1.getArtist())
            );

The same thing happens as in, for the TreeSet, there are no compilation errors but for Objects.compare the last call to thenComparing shows an error.

Can anyone please explain why this is happening and also why there is no need to use (Song p1) at all when simply calling the comparing method (without further thenComparing calls).

One other query on the same topic is when I do this to the TreeSet:

Set<Song> set = new TreeSet<Song>(
            Comparator.comparing(p1 -> p1.getTitle())
            .thenComparing(p1 -> p1.getDuration())
            .thenComparing(p1 -> p1.getArtist())
            );

i.e. remove the type Song from the first lambda parameter for the comparing method call, it shows syntax errors under the call to comparing and the first call to thenComparing but not to the final call to thenComparing - almost the opposite of what was happening above! Whereas, for all the other 3 examples i.e. with Objects.compare, List.sort and Collections.sort when I remove that first Song param type it shows syntax errors for all the calls.

Edited to include screenshot of errors I was receiving in Eclipse Kepler SR2, which I have now since found are Eclipse specific because when compiled using the JDK8 java compiler on the command-line it compiles OK.

Sort errors in Eclipse

starball
  • 20,030
  • 7
  • 43
  • 238
Tranquility
  • 3,061
  • 5
  • 23
  • 37
  • It would be helpful if you include in your question all the compilation error messages you are getting in all your tests. – Eran Jun 26 '14 at 17:43
  • 1
    to be honest, I think it would be easiest for someone to see what the issues are by running the source code themselves. – Tranquility Jun 26 '14 at 17:59
  • What are the types of `t1` and `t2` in the `Objects.compare` example? I'm trying to infer them, but layering my type inference over the compiler's type inference is intractable. :-) – Stuart Marks Jun 26 '14 at 18:31
  • 1
    Also what compiler are you using? – Stuart Marks Jun 26 '14 at 18:33
  • Yeah, the error with `Comparator.sort` happens with Eclipse's, but not the JDK's compiler. – Sotirios Delimanolis Jun 26 '14 at 18:34
  • I am using Eclipse Kepler SR2 with patches for Java 8. Also, t1 and t2 are of type Song, sorry, should have posted: Song t1 = new Song("Only girl in world", 238, "Rhianna"); Song t2 = new Song("Other song", 99, "Rhianna"); – Tranquility Jun 26 '14 at 18:53
  • Stuart Marks - another question I have for you is does the comparing and thenComparing methods use the compareTo method on the value passed to them, e.g. if a string is passed p1.getTitle() then I assume the compareTo method is used to compare the two title strings? I realised this by creating a field in my Song class of a custom type, e.g. class Name then trying to pass this to the comparator, e.g. comparing((Song p1) -> p1.getName()) and found it gave me a syntax error until I made the Name class implement Comparable. The javadoc is not clear on this. – Tranquility Jun 26 '14 at 18:58
  • @user3780370 yes the 2nd generic type in the `comparing` and `thenComparing` must implement `Comparable` unless you call the overloaded methods which take a 2nd function to do the comparison. – dkatzel Jun 26 '14 at 19:11
  • 1
    You've got two separate issues going here. One of the answerers pointed out you could use method references, which you sorta brushed off. Just as lambdas come in both "explicitly typed" and "implicitly typed" flavors, method references come in "exact" (one overload) and "inexact" (multiple overloads) flavors. Either an exact method ref or an explicit lambda can be used to provide additional typing information if not present. (Explicit type witnesses and casts can also be used, but are often bigger hammers.) – Brian Goetz Jun 26 '14 at 19:36
  • Note to future readers: Do not post images of error messages. Post the error message as text. [See here](https://meta.stackoverflow.com/a/285557/11107541) for why. – starball Dec 18 '22 at 22:51

4 Answers4

135

First, all the examples you say cause errors compile fine with the reference implementation (javac from JDK 8.) They also work fine in IntelliJ, so its quite possible the errors you're seeing are Eclipse-specific.

Your underlying question seems to be: "why does it stop working when I start chaining." The reason is, while lambda expressions and generic method invocations are poly expressions (their type is context-sensitive) when they appear as method parameters, when they appear instead as method receiver expressions, they are not.

When you say

Collections.sort(playlist1, comparing(p1 -> p1.getTitle()));

there is enough type information to solve for both the type argument of comparing() and the argument type p1. The comparing() call gets its target type from the signature of Collections.sort, so it is known comparing() must return a Comparator<Song>, and therefore p1 must be Song.

But when you start chaining:

Collections.sort(playlist1,
                 comparing(p1 -> p1.getTitle())
                     .thenComparing(p1 -> p1.getDuration())
                     .thenComparing(p1 -> p1.getArtist()));

now we've got a problem. We know that the compound expression comparing(...).thenComparing(...) has a target type of Comparator<Song>, but because the receiver expression for the chain, comparing(p -> p.getTitle()), is a generic method call, and we can't infer its type parameters from its other arguments, we're kind of out of luck. Since we don't know the type of this expression, we don't know that it has a thenComparing method, etc.

There are several ways to fix this, all of which involve injecting more type information so that the initial object in the chain can be properly typed. Here they are, in rough order of decreasing desirability and increasing intrusiveness:

  • Use an exact method reference (one with no overloads), like Song::getTitle. This then gives enough type information to infer the type variables for the comparing() call, and therefore give it a type, and therefore continue down the chain.
  • Use an explicit lambda (as you did in your example).
  • Provide a type witness for the comparing() call: Comparator.<Song, String>comparing(...).
  • Provide an explicit target type with a cast, by casting the receiver expression to Comparator<Song>.
Andrew Gaul
  • 2,296
  • 1
  • 12
  • 19
Brian Goetz
  • 90,105
  • 23
  • 150
  • 161
  • 17
    +1 for actually answering the OP "why can't the compiler infer this" rather than just giving workarounds/solutions. – Joffrey Jun 27 '14 at 08:38
  • Thank you for your answer Brian. However, I still find something unanswered, why does List.sort behave differently to Collections.sort, in that the former only requires the first lambda to contain the parameter type, but the latter also requires the last to, e.g. if I have a comparing followed by 5 thenComparing calls I would have to put (Song p1) in the comparing and in the last thenComparing. Also in my original post you will see the bottom example of the TreeSet where I remove all param types and yet the last call to thenComparing is OK but the others are not - so this behaves differently. – Tranquility Jun 27 '14 at 12:41
  • 3
    @user3780370 Are you still using the Eclipse compiler? I haven't seen this behavior, if I understand your question correctly. Can you (a) try it with javac from JDK 8 and (b) if it still fails, post the code? – Brian Goetz Jun 27 '14 at 15:01
  • 1
    @BrianGoetz Thanks for this suggestion. I have just compiled it within the Command Window using javac and it compiles as you said. It appears to be an Eclipse issue. I've not yet updated to Eclipse Luna, which is purpose-built for JDK8 so hopefully it may be fixed in that. I actually have a screenshot to show you what was happening in Eclipse but don't know how to post on here. – Tranquility Jun 29 '14 at 16:00
  • 2
    I think you mean `Comparator.comparing(...)`. – shmosel Feb 09 '17 at 03:40
26

The problem is type inferencing. Without adding a (Song s) to the first comparison, comparator.comparing doesn't know the type of the input so it defaults to Object.

You can fix this problem 1 of 3 ways:

  1. Use the new Java 8 method reference syntax

     Collections.sort(playlist,
                Comparator.comparing(Song::getTitle)
                .thenComparing(Song::getDuration)
                .thenComparing(Song::getArtist)
                );
    
  2. Pull out each comparison step into a local reference

      Comparator<Song> byName = (s1, s2) -> s1.getArtist().compareTo(s2.getArtist());
    
      Comparator<Song> byDuration = (s1, s2) -> Integer.compare(s1.getDuration(), s2.getDuration());
    
        Collections.sort(playlist,
                byName
                .thenComparing(byDuration)
                );
    

    EDIT

  3. Forcing the type returned by the Comparator (note you need both the input type and the comparison key type)

    sort(
      Comparator.<Song, String>comparing((s) -> s.getTitle())
                .thenComparing(p1 -> p1.getDuration())
                .thenComparing(p1 -> p1.getArtist())
                );
    

I think the "last" thenComparing syntax error is misleading you. It's actually a type problem with the whole chain, it's just the compiler only marking the end of the chain as a syntax error because that's when the final return type doesn't match I guess.

I'm not sure why List is doing a better inferencing job than Collection since it should do the same capture type but apparently not.

dkatzel
  • 31,188
  • 3
  • 63
  • 67
  • Why does it know it for the `ArrayList` but not for the `Collections` solution (given the first call in the chain has a `Song` parameter)? – Sotirios Delimanolis Jun 26 '14 at 18:10
  • 4
    Thank you for your reply, however, if you read my post you would see I said: "Before we start, I know I could use method references, e.g. Song::getTitle to overcome my problems, but my query here is not so much something I want to fix but something I want an answer to, i.e. why is the Java compiler handling it in this way." – Tranquility Jun 26 '14 at 18:10
  • I want an answer as to why the compiler is behaving that way when I use lambda expressions. It accepts comparing(s -> s.getArtist()) but then when I chain .thenComparing(s -> s.getDuration()) for example it gives me syntax errors for both calls, if I then add a an explicit type in the comparing call, e.g. comparing((Song s) -> s.getArtist()) then this fixes that problem and for the List.sort and TreeSet it also solves all further compilation errors without having to add additional param types, however, for the Collections.sort & Objects.compare examples the last thenComparing still fails – Tranquility Jun 26 '14 at 18:14
2

Another way to deal with this compile time error:

Cast your first comparing function's variable explicitly and then good to go. I have sort the list of org.bson.Documents object. Please look at sample code

Comparator<Document> comparator = Comparator.comparing((Document hist) -> (String) hist.get("orderLineStatus"), reverseOrder())
                       .thenComparing(hist -> (Date) hist.get("promisedShipDate"))
                       .thenComparing(hist -> (Date) hist.get("lastShipDate"));
list = list.stream().sorted(comparator).collect(Collectors.toList());
0

playlist1.sort(...) creates a bound of Song for the type variable E, from the declaration of playlist1, which "ripples" to the comparator.

In Collections.sort(...), there is no such bound, and the inference from the type of the first comparator is not enough for the compiler to infer the rest.

I think you would get "correct" behavior from Collections.<Song>sort(...), but don't have a java 8 install to test it out for you.

amalloy
  • 89,153
  • 8
  • 140
  • 205