Collectors.groupingBy doesn't accept null keys

Question

In Java 8, this works:

Stream<Class> stream = Stream.of(ArrayList.class);
HashMap<Class, List<Class>> map = (HashMap)stream.collect(Collectors.groupingBy(Class::getSuperclass));

But this doesn't:

Stream<Class> stream = Stream.of(List.class);
HashMap<Class, List<Class>> map = (HashMap)stream.collect(Collectors.groupingBy(Class::getSuperclass));

Maps allows a null key, and List.class.getSuperclass() returns null. But Collectors.groupingBy emits a NPE, at Collectors.java, line 907:

K key = Objects.requireNonNull(classifier.apply(t), "element cannot be mapped to a null key"); 

It works if I create my own collector, with this line changed to:

K key = classifier.apply(t);  

My questions are:

1) The Javadoc of Collectors.groupingBy doesn't say it shouldn't map a null key. Is this behavior necessary for some reason?

2) Is there another, easier way, to accept a null key, without having to create my own collector?

Solution 1

For the first question, I agree with skiwi that it shouldn't be throwing a NPE. I hope they will change that (or else at least add it to the javadoc). Meanwhile, to answer the second question I decided to use Collectors.toMap instead of Collectors.groupingBy:

Stream<Class<?>> stream = Stream.of(ArrayList.class);

Map<Class<?>, List<Class<?>>> map = stream.collect(
    Collectors.toMap(
        Class::getSuperclass,
        Collections::singletonList,
        (List<Class<?>> oldList, List<Class<?>> newEl) -> {
        List<Class<?>> newList = new ArrayList<>(oldList.size() + 1);
        newList.addAll(oldList);
        newList.addAll(newEl);
        return newList;
        }));

Or, encapsulating it:

/** Like Collectors.groupingBy, but accepts null keys. */
public static <T, A> Collector<T, ?, Map<A, List<T>>>
groupingBy_WithNullKeys(Function<? super T, ? extends A> classifier) {
    return Collectors.toMap(
        classifier,
        Collections::singletonList,
        (List<T> oldList, List<T> newEl) -> {
            List<T> newList = new ArrayList<>(oldList.size() + 1);
            newList.addAll(oldList);
            newList.addAll(newEl);
            return newList;
            });
    }

And use it like this:

Stream<Class<?>> stream = Stream.of(ArrayList.class);
Map<Class<?>, List<Class<?>>> map = stream.collect(groupingBy_WithNullKeys(Class::getSuperclass));

Please note rolfl gave another, more complicated answer, which allows you to specify your own Map and List supplier. I haven't tested it.

Solution 2

I had the same kind of problem. This failed, because groupingBy performs Objects.requireNonNull on the value returned from the classifier:

    Map<Long, List<ClaimEvent>> map = events.stream()
      .filter(event -> eventTypeIds.contains(event.getClaimEventTypeId()))
      .collect(groupingBy(ClaimEvent::getSubprocessId));

Using Optional, this works:

    Map<Optional<Long>, List<ClaimEvent>> map = events.stream()
      .filter(event -> eventTypeIds.contains(event.getClaimEventTypeId()))
      .collect(groupingBy(event -> Optional.ofNullable(event.getSubprocessId())));

Solution 3

Use filter before groupingBy##

Filter out the null instances before groupingBy.

MyObjectlist.stream()
  .filter(p -> p.getSomeInstance() != null)
  .collect(Collectors.groupingBy(MyObject::getSomeInstance));

Solution 4

To your 1st question, from the docs:

There are no guarantees on the type, mutability, serializability, or thread-safety of the Map or List objects returned.

Because not all Map implementations allow null keys they probably added this to reduce to the most common allowable definition of a map to get maximum flexibility when choosing a type.

To your 2nd question, you just need a supplier, wouldn't a lambda work? I'm still getting acquainted with Java 8, maybe a smarter person can add a better answer.

Solution 5

First of all, you are using lots of raw objects. This is not a good idea at all, first convert the following:

• Class to Class<?>, ie. instead of a raw type, a parametrized type with an unknown class.
• Instead of forcefully casting to a HashMap, you should supply a HashMap to the collector.

First the correctly typed code, without caring about a NPE yet:

Stream<Class<?>> stream = Stream.of(ArrayList.class);
HashMap<Class<?>, List<Class<?>>> hashMap = (HashMap<Class<?>, List<Class<?>>>)stream
        .collect(Collectors.groupingBy(Class::getSuperclass));

Now we get rid of the forceful cast there, and instead do it correctly:

Stream<Class<?>> stream = Stream.of(ArrayList.class);
HashMap<Class<?>, List<Class<?>>> hashMap = stream
        .collect(Collectors.groupingBy(
                Class::getSuperclass,
                HashMap::new,
                Collectors.toList()
        ));

Here we replace the groupingBy which just takes a classifier, to one that takes a classifier, a supplier and a collector. Essentially this is the same as what there was before, but now it is correctly typed.

You are indeed correct that in the javadoc it is not stated that it will throw a NPE, and I do not think it should be throwing one, as I am allowed to supply whatever map I want, and if my map allows null keys, then it should be allowed.

I do not see any other way to do it simpler as of now, I'll try to look more into it.

View more solutions

Comments

[skiwi]

You can specify a map application that does accept null keys, so yes it is quite a big issue that it will never accept null keys.

[MarcG]

Skiwi, I typed a bad example, since I don't really need a HashMap, and the simpler groupingBy is not guaranteed to return one. My example should have been just Map, like this: Stream<Class<?>> stream = Stream.of(ArrayList.class); Map<Class<?>, List<Class<?>>> map = stream.collect(Collectors.groupingBy(Class::getSuperclass));

[MarcG]

A warning for IntellijIDEA13 users: the IDE issues a Cyclic inference error on Class::getSuperclass. If you remove all the <?> this error is not shown. But now I realized it compiles and runs even with this "error", so it seems not to be a real error, but an IntellijIDEA bug.

[MarcG]

Jason, the Map interface allows it to accept or reject nulls. So if they want maximum flexibility they should accept them. Anyway, if someone uses a map implementation that doesn't accept nulls, it can throw its own exceptions.

[Jason Sperske]

The interface does but the implementations do not all support null keys. By preventing nulls in the generic collector they have the flexibility to choose any map implementation. I agree that this feels off but likely they weighed the possibility of a mysterious null pointer exception based on performance tuning (or however it chooses an implementation) verses the widest (as in works with all implementations of the Map interface) definition of a key.

[pdem]

Thanks, it helped me for a different need: groupBy and Count, here is my wrapper: public static <T,R> Map<R,Long> countGroupBy(Collection<T> list, Function<T,R> groupBy){ Collector<T, ?, Map<R, Long>> mapCollectors = Collectors.toMap( groupBy, l->1L, Long::sum); return list.stream().collect(mapCollectors); }

[Clint Eastwood]

The second question from the OPS implicitly states that they want to accept null keys so this doesn't answer his question.

[Clint Eastwood]

this answer doesn't work. All groupingBy methods in Collectors end up using internally the same implementation that rejects null keys. Can't believe nobody has bothered to check this before upvoting...

[Speedstone]

This can be a part of a correct answer. The other part being that you can then explicitly put null items in the result: result.put(null, myList.stream.filter(p -> p.get() == null).collect(Collectors.toList());Kind of ugly, but may still be more convenient than creating a collector.

[Daniel Alder]

The problem: the groupingBy lambda function has to be called twice. this could be expensive, depending on the function

[Kirill Gagarski]

The other problem is that groupingBy result is not guaranteed to be mutable. But you can pass mapFactory parameter, supplying a mutable map (like HashMap::new)