Search code examples
javajava-streamcomparator

Comparator - thenComparing() method produces 'no instance(s) of type variable(s) U exist so that Object conforms to Comparable<? super U>'


What is a proper usage of the Comparator's methodthenComparing() and why it's not working properly in my code? I don't quite understand the reason of the error I'm getting:

no instance(s) of type variable(s) U exist so 
that Object conforms to Comparable<? super U>

That is the code that produces the error:

Map<Integer, Long> sortedCards = new LinkedHashMap<>();
List<Card> cards = // initializing the list

cards.stream().collect(
    Collectors.groupingBy(
        card -> card.kind.rank,
        Collectors.counting()
    ))
    .entrySet().stream()
    .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder())
                     .thenComparing(Map.Entry::getKey))
    .forEachOrdered(e -> sortedCards.put(e.getKey(), e.getValue()));

My Card class:

public static class Card implements Comparable<Card> {

    private Kind kind;
    
    // constructors, getters, etc.
}

Kind enum:

public enum Kind {

    TWO(1, "2"), THREE(2, "3"), FOUR(3, "4"), // etc.;
    
    public int rank;
    // constructors, getters, etc.
}

Solution

  • The reason of the compilation error you've encountered is a type inference issue.

    The when you're chaining methods while constructing a comparator, the compiler fails to infer the type of the method reference from the target type.

    You can resolve it by using type-witness, i.e. providing the types of key and value explosively in the angle brackets <K,V> (these are the types declared by comparingByValue() which produces the first comparator in the chain):

    Map.Entry.<Integer, Long>comparingByValue(Comparator.reverseOrder())
        .thenComparing(Map.Entry::getKey)
    

    Also, instead of forEachOrdered() it would be better to use collect() and generate a Map as a result of the stream execution then populate pre-created map via side-effects. It makes your code more verbose and less expressive because you need to instantiate the resulting collection separately, and goes against the guidelines listed in the documentation.

    Methods forEach() and forEachOrdered() exist as a last resort, and it's discouraged to use them as a substitution of reduction operations like collect() or reduce(), for more information refer to the API documentation, pay attention to the code examples.

    That's how it would look like if we make use of collect():

    Map<Integer, Long> sortedCards = cards.stream()
        .collect(Collectors.groupingBy(card -> card.kind.rank, Collectors.counting()))
        .entrySet().stream()
        .sorted(Map.Entry.<Integer, Long>comparingByValue(Comparator.reverseOrder())
            .thenComparing(Map.Entry::getKey))
        .collect(Collectors.toMap(
            Map.Entry::getKey,
            Map.Entry::getValue,
            (left, right) -> {
                throw new AssertionError("duplicate keys are not expected");
            },
            LinkedHashMap::new
        ));