NullPointerException in Collectors.toMap with null entry values

Question

Collectors.toMap throws a NullPointerException if one of the values is null. I don't understand this behaviour, maps can contain null pointers as value without any problems. Is there a good reason why values cannot be null for Collectors.toMap?

Also, is there a nice Java 8 way of fixing this, or should I revert to plain old for loop?

An example of my problem:

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;


class Answer {
    private int id;

    private Boolean answer;

    Answer() {
    }

    Answer(int id, Boolean answer) {
        this.id = id;
        this.answer = answer;
    }

    public int getId() {
        return id;
    }

    public void setId(int id) {
        this.id = id;
    }

    public Boolean getAnswer() {
        return answer;
    }

    public void setAnswer(Boolean answer) {
        this.answer = answer;
    }
}

public class Main {
    public static void main(String[] args) {
        List<Answer> answerList = new ArrayList<>();

        answerList.add(new Answer(1, true));
        answerList.add(new Answer(2, true));
        answerList.add(new Answer(3, null));

        Map<Integer, Boolean> answerMap =
        answerList
                .stream()
                .collect(Collectors.toMap(Answer::getId, Answer::getAnswer));
    }
}

Stacktrace:

Exception in thread "main" java.lang.NullPointerException
    at java.util.HashMap.merge(HashMap.java:1216)
    at java.util.stream.Collectors.lambda$toMap$168(Collectors.java:1320)
    at java.util.stream.Collectors$$Lambda$5/1528902577.accept(Unknown Source)
    at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1359)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    at Main.main(Main.java:48)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

This problem still exists in Java 11.

Answer 1

You can work around this known bug in OpenJDK with this:

Map<Integer, Boolean> collect = list.stream()
        .collect(HashMap::new, (m,v)->m.put(v.getId(), v.getAnswer()), HashMap::putAll);

It is not that much pretty, but it works. Result:

1: true
2: true
3: null

(this tutorial helped me the most.)

EDIT:

Unlike Collectors.toMap, this will silently replace values if you have the same key multiple times, as @mmdemirbas pointed out in the comments. If you don't want this, look at the link in the comment.

Answer 2

It is not possible with the static methods of Collectors. The javadoc of toMap explains that toMap is based on Map.merge:

@param mergeFunction a merge function, used to resolve collisions between values associated with the same key, as supplied to Map#merge(Object, Object, BiFunction)}

and the javadoc of Map.merge says:

@throws NullPointerException if the specified key is null and this map does not support null keys or the value or remappingFunction is null

You can avoid the for loop by using the forEach method of your list.

Map<Integer,  Boolean> answerMap = new HashMap<>();
answerList.forEach((answer) -> answerMap.put(answer.getId(), answer.getAnswer()));

but it is not really simple than the old way:

Map<Integer, Boolean> answerMap = new HashMap<>();
for (Answer answer : answerList) {
    answerMap.put(answer.getId(), answer.getAnswer());
}

Answer 3

I wrote a Collector which, unlike the default java one, does not crash when you have null values:

public static <T, K, U>
        Collector<T, ?, Map<K, U>> toMap(Function<? super T, ? extends K> keyMapper,
                Function<? super T, ? extends U> valueMapper) {
    return Collectors.collectingAndThen(
            Collectors.toList(),
            list -> {
                Map<K, U> result = new HashMap<>();
                for (T item : list) {
                    K key = keyMapper.apply(item);
                    if (result.putIfAbsent(key, valueMapper.apply(item)) != null) {
                        throw new IllegalStateException(String.format("Duplicate key %s", key));
                    }
                }
                return result;
            });
}

Just replace your Collectors.toMap() call to a call to this function and it'll fix the problem.

Answer 4

Yep, a late answer from me, but I think it may help to understand what's happening under the hood in case anyone wants to code some other Collector-logic.

I tried to solve the problem by coding a more native and straight forward approach. I think it's as direct as possible:

public class LambdaUtilities {

  /**
   * In contrast to {@link Collectors#toMap(Function, Function)} the result map
   * may have null values.
   */
  public static <T, K, U, M extends Map<K, U>> Collector<T, M, M> toMapWithNullValues(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper) {
    return toMapWithNullValues(keyMapper, valueMapper, HashMap::new);
  }

  /**
   * In contrast to {@link Collectors#toMap(Function, Function, BinaryOperator, Supplier)}
   * the result map may have null values.
   */
  public static <T, K, U, M extends Map<K, U>> Collector<T, M, M> toMapWithNullValues(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper, Supplier<Map<K, U>> supplier) {
    return new Collector<T, M, M>() {

      @Override
      public Supplier<M> supplier() {
        return () -> {
          @SuppressWarnings("unchecked")
          M map = (M) supplier.get();
          return map;
        };
      }

      @Override
      public BiConsumer<M, T> accumulator() {
        return (map, element) -> {
          K key = keyMapper.apply(element);
          if (map.containsKey(key)) {
            throw new IllegalStateException("Duplicate key " + key);
          }
          map.put(key, valueMapper.apply(element));
        };
      }

      @Override
      public BinaryOperator<M> combiner() {
        return (left, right) -> {
          int total = left.size() + right.size();
          left.putAll(right);
          if (left.size() < total) {
            throw new IllegalStateException("Duplicate key(s)");
          }
          return left;
        };
      }

      @Override
      public Function<M, M> finisher() {
        return Function.identity();
      }

      @Override
      public Set<Collector.Characteristics> characteristics() {
        return Collections.unmodifiableSet(EnumSet.of(Collector.Characteristics.IDENTITY_FINISH));
      }

    };
  }

}

And the tests using JUnit and assertj:

  @Test
  public void testToMapWithNullValues() throws Exception {
    Map<Integer, Integer> result = Stream.of(1, 2, 3)
        .collect(LambdaUtilities.toMapWithNullValues(Function.identity(), x -> x % 2 == 1 ? x : null));

    assertThat(result)
        .isExactlyInstanceOf(HashMap.class)
        .hasSize(3)
        .containsEntry(1, 1)
        .containsEntry(2, null)
        .containsEntry(3, 3);
  }

  @Test
  public void testToMapWithNullValuesWithSupplier() throws Exception {
    Map<Integer, Integer> result = Stream.of(1, 2, 3)
        .collect(LambdaUtilities.toMapWithNullValues(Function.identity(), x -> x % 2 == 1 ? x : null, LinkedHashMap::new));

    assertThat(result)
        .isExactlyInstanceOf(LinkedHashMap.class)
        .hasSize(3)
        .containsEntry(1, 1)
        .containsEntry(2, null)
        .containsEntry(3, 3);
  }

  @Test
  public void testToMapWithNullValuesDuplicate() throws Exception {
    assertThatThrownBy(() -> Stream.of(1, 2, 3, 1)
        .collect(LambdaUtilities.toMapWithNullValues(Function.identity(), x -> x % 2 == 1 ? x : null)))
            .isExactlyInstanceOf(IllegalStateException.class)
            .hasMessage("Duplicate key 1");
  }

  @Test
  public void testToMapWithNullValuesParallel() throws Exception {
    Map<Integer, Integer> result = Stream.of(1, 2, 3)
        .parallel() // this causes .combiner() to be called
        .collect(LambdaUtilities.toMapWithNullValues(Function.identity(), x -> x % 2 == 1 ? x : null));

    assertThat(result)
        .isExactlyInstanceOf(HashMap.class)
        .hasSize(3)
        .containsEntry(1, 1)
        .containsEntry(2, null)
        .containsEntry(3, 3);
  }

  @Test
  public void testToMapWithNullValuesParallelWithDuplicates() throws Exception {
    assertThatThrownBy(() -> Stream.of(1, 2, 3, 1, 2, 3)
        .parallel() // this causes .combiner() to be called
        .collect(LambdaUtilities.toMapWithNullValues(Function.identity(), x -> x % 2 == 1 ? x : null)))
            .isExactlyInstanceOf(IllegalStateException.class)
            .hasCauseExactlyInstanceOf(IllegalStateException.class)
            .hasStackTraceContaining("Duplicate key");
  }

And how do you use it? Well, just use it instead of toMap() like the tests show. This makes the calling code look as clean as possible.

EDIT:
implemented Holger's idea below, added a test method

Answer 5

Here's somewhat simpler collector than proposed by @EmmanuelTouzery. Use it if you like:

public static <T, K, U> Collector<T, ?, Map<K, U>> toMapNullFriendly(
        Function<? super T, ? extends K> keyMapper,
        Function<? super T, ? extends U> valueMapper) {
    @SuppressWarnings("unchecked")
    U none = (U) new Object();
    return Collectors.collectingAndThen(
            Collectors.<T, K, U> toMap(keyMapper,
                    valueMapper.andThen(v -> v == null ? none : v)), map -> {
                map.replaceAll((k, v) -> v == none ? null : v);
                return map;
            });
}

We just replace null with some custom object none and do the reverse operation in the finisher.

Answer 6

I have slightly modified Emmanuel Touzery's null-safe map Collector implementation.

This version:

• Allows null keys
• Allows null values
• Detects duplicate keys (even if they are null) and throws IllegalStateException as in the original JDK implementation
• Detects duplicate keys also when the key already mapped to the null value. In other words, separates a mapping with null-value from no-mapping

public static <T, K, U> Collector<T, ?, Map<K, U>> toMapOfNullables(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper) {
    return Collectors.collectingAndThen(
        Collectors.toList(),
        list -> {
            Map<K, U> map = new LinkedHashMap<>();
            list.forEach(item -> {
                K key = keyMapper.apply(item);
                U value = valueMapper.apply(item);
                if (map.containsKey(key)) {
                    throw new IllegalStateException(String.format(
                            "Duplicate key %s (attempted merging values %s and %s)",
                            key, map.get(key), value));
                }
                map.put(key, value);
            });
            return map;
        }
    );
}

Unit tests:

@Test
public void toMapOfNullables_WhenHasNullKey() {
    assertEquals(singletonMap(null, "value"),
        Stream.of("ignored").collect(Utils.toMapOfNullables(i -> null, i -> "value"))
    );
}

@Test
public void toMapOfNullables_WhenHasNullValue() {
    assertEquals(singletonMap("key", null),
        Stream.of("ignored").collect(Utils.toMapOfNullables(i -> "key", i -> null))
    );
}

@Test
public void toMapOfNullables_WhenHasDuplicateNullKeys() {
    assertThrows(new IllegalStateException("Duplicate key null"),
        () -> Stream.of(1, 2, 3).collect(Utils.toMapOfNullables(i -> null, i -> i))
    );
}

@Test
public void toMapOfNullables_WhenHasDuplicateKeys_NoneHasNullValue() {
    assertThrows(new IllegalStateException("Duplicate key duplicated-key"),
        () -> Stream.of(1, 2, 3).collect(Utils.toMapOfNullables(i -> "duplicated-key", i -> i))
    );
}

@Test
public void toMapOfNullables_WhenHasDuplicateKeys_OneHasNullValue() {
    assertThrows(new IllegalStateException("Duplicate key duplicated-key"),
        () -> Stream.of(1, null, 3).collect(Utils.toMapOfNullables(i -> "duplicated-key", i -> i))
    );
}

@Test
public void toMapOfNullables_WhenHasDuplicateKeys_AllHasNullValue() {
    assertThrows(new IllegalStateException("Duplicate key duplicated-key"),
        () -> Stream.of(null, null, null).collect(Utils.toMapOfNullables(i -> "duplicated-key", i -> i))
    );
}

Answer 7

If the value is a String, then this might work: map.entrySet().stream().collect(Collectors.toMap(e -> e.getKey(), e -> Optional.ofNullable(e.getValue()).orElse("")))

Answer 8

According to the Stacktrace

Exception in thread "main" java.lang.NullPointerException
at java.util.HashMap.merge(HashMap.java:1216)
at java.util.stream.Collectors.lambda$toMap$148(Collectors.java:1320)
at java.util.stream.Collectors$$Lambda$5/391359742.accept(Unknown Source)
at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1359)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
at com.guice.Main.main(Main.java:28)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:483)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:134)

When is called the map.merge

        BiConsumer<M, T> accumulator
            = (map, element) -> map.merge(keyMapper.apply(element),
                                          valueMapper.apply(element), mergeFunction);

It will do a null check as first thing

if (value == null)
    throw new NullPointerException();

I don't use Java 8 so often so i don't know if there are a better way to fix it, but fix it is a bit hard.

You could do:

Use filter to filter all NULL values, and in the Javascript code check if the server didn't send any answer for this id means that he didn't reply to it.

Something like this:

Map<Integer, Boolean> answerMap =
        answerList
                .stream()
                .filter((a) -> a.getAnswer() != null)
                .collect(Collectors.toMap(Answer::getId, Answer::getAnswer));

Or use peek, which is used to alter the stream element for element. Using peek you could change the answer to something more acceptable for map but it means edit your logic a bit.

Sounds like if you want to keep the current design you should avoid Collectors.toMap

Answer 9

public static <T, K, V> Collector<T, HashMap<K, V>, HashMap<K, V>> toHashMap(
        Function<? super T, ? extends K> keyMapper,
        Function<? super T, ? extends V> valueMapper
)
{
    return Collector.of(
            HashMap::new,
            (map, t) -> map.put(keyMapper.apply(t), valueMapper.apply(t)),
            (map1, map2) -> {
                map1.putAll(map2);
                return map1;
            }
    );
}

public static <T, K> Collector<T, HashMap<K, T>, HashMap<K, T>> toHashMap(
        Function<? super T, ? extends K> keyMapper
)
{
    return toHashMap(keyMapper, Function.identity());
}

Answer 10

Sorry to reopen an old question, but since it was edited recently saying that the "issue" still remains in Java 11, I felt like I wanted to point out this:

answerList
        .stream()
        .collect(Collectors.toMap(Answer::getId, Answer::getAnswer));

gives you the null pointer exception because the map does not allow null as a value. This makes sense because if you look in a map for the key k and it is not present, then the returned value is already null (see javadoc). So if you were able to put in k the value null, the map would look like it's behaving oddly.

As someone said in the comments, it's pretty easy to solve this by using filtering:

answerList
        .stream()
        .filter(a -> a.getAnswer() != null)
        .collect(Collectors.toMap(Answer::getId, Answer::getAnswer));

in this way no null values will be inserted in the map, and STILL you will get null as the "value" when looking for an id that does not have an answer in the map.

I hope this makes sense to everyone.

Answer 11

For completeness, I'm posting a version of the toMapOfNullables with a mergeFunction param:

public static <T, K, U> Collector<T, ?, Map<K, U>> toMapOfNullables(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper, BinaryOperator<U> mergeFunction) {
    return Collectors.collectingAndThen(Collectors.toList(), list -> {
        Map<K, U> result = new HashMap<>();
        for(T item : list) {
            K key = keyMapper.apply(item);
            U newValue = valueMapper.apply(item);
            U value = result.containsKey(key) ? mergeFunction.apply(result.get(key), newValue) : newValue;
            result.put(key, value);
        }
        return result;
    });
}

Answer 12

Retaining all questions ids with small tweak

Map<Integer, Boolean> answerMap = 
  answerList.stream()
            .collect(Collectors.toMap(Answer::getId, a -> 
                       Boolean.TRUE.equals(a.getAnswer())));