2

The following code won't compile:

  public static Map<String, Collection<String>> convert(Collection<Foo> foos) {
    return foos == null ? Maps.newHashMap()
                        : foos.stream().collect(
                            Collectors.groupingBy(
                                f -> f.getSomeEnum().name(),
                                Collectors.mapping(Foo::getVal, Collectors.toList())));
  }

Unless I change the return type to List:

  public static Map<String, List<String>> convert(Collection<Foo> foos) {

Normally I believe I should be able to, but maybe there is some ambiguity introduced by generics?

The exact error:

Incompatible types. Required Map<String, Collection<String>> but 'collect' was inferred to R: no instance(s) of type variable(s) A, A, A, A, A, A, D, K, R, R, T, T, T, U exist so that List<T> conforms to Collection<String>

I don't think the details are relevant but just in case:

Foo is like this:

public class Foo {
  private MyEnumType someEnum;
  private String val;
  public MyEnumType getSomeEnum();
  public String getVal();
}

and I am trying to convert a list of Foos to a map of Foos, vals grouped by someEnum.

Andrew Cheong
  • 29,362
  • 15
  • 90
  • 145

2 Answers2

4

Map<String, List<String>> is not a subtype of Map<String, Collection<String>>, see this for more.

You can declare the return type as Map<String, ? extends Collection<String>>

xingbin
  • 27,410
  • 9
  • 53
  • 103
2

The method signatures of groupingBy and mapping have no variance regarding the result type of the downstream collector. Hence, you end up with the result type List<T> of the toList() collector. In contrast, toCollection has variance in its type parameter C extends Collection<T>, but even without, there wouldn’t be any problem to assign ArrayList::new to a Supplier<Collection<…>>:

public static Map<String, Collection<String>> convert(Collection<Foo> foos) {
    return foos == null? new HashMap<>(): foos.stream().collect(
        Collectors.groupingBy(f -> f.getSomeEnum().name(),
            Collectors.mapping(Foo::getVal, Collectors.toCollection(ArrayList::new))));
}

This does exactly the same as toList(), given the current implementation. But it wouldn’t benefit from future improvements specifically made to the toList() collector. An alternative would be to keep using toList(), but chain a type conversion:

public static Map<String, Collection<String>> convert(Collection<Foo> foos) {
    return foos == null? new HashMap<>(): foos.stream().collect(
        Collectors.groupingBy(f -> f.getSomeEnum().name(),
            Collectors.collectingAndThen(
                Collectors.mapping(Foo::getVal, Collectors.toList()),
                c -> c)));
}

Since this is a widening conversion, the conversion function is as simple as c -> c. Unfortunately, the underlying implementation doesn’t know about the triviality of this function and will iterate over the result map’s values to replace each of them with the result of applying this function.

This can be solved with a special widening collector:

public static <T,A,R extends W,W> Collector<T,A,W> wideningResult(Collector<T,A,R> original) {
    return Collector.of(original.supplier(), original.accumulator(), original.combiner(),
        original.finisher().andThen(t -> t),
        original.characteristics().toArray(new Collector.Characteristics[0]));
}

This does basically the same as Collectors.collectingAndThen(original, t -> t), chaining the trivial widening conversion function. But it keeps the original collector’s characteristics, so if the original collector has the IDENTITY_FINISH characteristic, we will still have it, which allows to skip the finishing operation, which in case of groupingBy implies that it doesn’t need to iterate over the map to apply the function.

Applying it to the actual use case yields

public static Map<String, Collection<String>> convert(Collection<Foo> foos) {
    return foos == null? new HashMap<>(): foos.stream().collect(
        Collectors.groupingBy(f -> f.getSomeEnum().name(),
            wideningResult(Collectors.mapping(Foo::getVal, Collectors.toList()))));
}
Holger
  • 285,553
  • 42
  • 434
  • 765
  • I did not even notice that `C extends Collection`... nice! And this trick with extra characteristics, well we use it too :) (it's in the package with your name, so I guess it might have been added by me also) for the case when *I know* an incoming List is sorted... – Eugene Jun 14 '18 at 08:13