NullPointerException in Collectors.toMap with null entry values
JavaNullpointerexceptionJava StreamCollectorsJava Problem Overview
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.
Java Solutions
Solution 1 - Java
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.
Solution 2 - Java
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());
}
Solution 3 - Java
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.
Solution 4 - Java
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.
Solution 5 - Java
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
Solution 6 - Java
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))
);
}
Solution 7 - Java
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("")))
Solution 8 - Java
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
Solution 9 - Java
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());
}
Solution 10 - Java
Retaining all questions ids with small tweak
Map<Integer, Boolean> answerMap =
answerList.stream()
.collect(Collectors.toMap(Answer::getId, a ->
Boolean.TRUE.equals(a.getAnswer())));
Solution 11 - Java
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.
Solution 12 - Java
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;
});
}