Collectors.toMap does not like null

 This article was originally published on JRoller on November 26, 2015

Some map accept null values, some don't. How do you know? You usually take a look in the javadoc. But what about maps created by streams through the Collectors.toMap? The javadoc does not say. So I tried out. I picked the following code:

 	List<String> player = Arrays.asList("Lebron", "Kobe", "Shaquille");
	List<String> team = Arrays.asList("Cleveland", "Los Angeles", null);
	
	Map<String, String> currentTeam = new HashMap<>();
	for (int i = 0; i < player.size(); i++) {
		currentTeam.put(player.get(i), team.get(i));
	}

Everything works as expected, it inserts the null value into my map. So I tried to convert it to streams (maybe not in the best way):

	Map<String, String> currentTeam = IntStream.range(0, player.size())
		.mapToObj(i -> i)
		.collect(Collectors.toMap(i -> player.get(i), i -> team.get(i)));

Here is what I get:

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$6/149928006.accept(Unknown Source)
	at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
	at java.util.stream.IntPipeline$4$1.accept(IntPipeline.java:250)
	at java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:110)
	at java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:693)
	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 Test.main(Test.java:23)

Is it using a kind of Map that does not accept null values? Let's check:

   public static <T, K, U>
    Collector<T, ?, Map<K,U>> toMap(Function<? super T, ? extends K> keyMapper,
                                    Function<? super T, ? extends U> valueMapper) {
        return toMap(keyMapper, valueMapper, throwingMerger(), HashMap::new);
    } 

Well, no. It uses a standard HashMap. In the stack, the Exception is thrown from the HashMap.merge() function. So let's have a look:

   @Override
    public V merge(K key, V value,
                   BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
        if (value == null)
            throw new NullPointerException();
	...
    }

So the problem is not in the type of map but in the implementation of the merge. The javadoc for the HashMap merge() method says that the value parameter is "the non-null value to be merged with the existing value associated with the key". So yes, it says it in the Javadoc, but not where you would expect it.

By the way, if you really want to make it work with streams, you would have to supply your own collector (as an aside note, the same problem arises with the groupBy collector) :

	Map<String, String> currentTeam = IntStream.range(0, player.size())
		.mapToObj(i -> i)
		.collect(HashMap::new,
			(map, i) -> map.put(player.get(i), team.get(i)),
			HashMap::putAll);

Maybe I'll stick with the loop for this time.

Lambda Before Switch

 This article was originally published in JRoller on September 22, 2015.

We had this method that was doing the same thing several times in a row. Here is a simplified version:

void resolveAll() {
	a = resolveA();
	if (a == null)
		states.remove(State.A);
	else
		states.add(State.A);

	b = resolveB();
	if (b == null)
		states.remove(State.B);
	else
		states.add(State.B);

	c = resolveC();
	if (c == null)
		states.remove(State.C);
	else
		states.add(State.C);
} 

It's almost the same thing repeated three times (a bit more in the real life code), except for this method resolveX which is different each time, returning a different type of object. We needed to modify the code, but did not feel like having to repeat the change several times. So we resorted to a refactoring. But what to do with this resolveX method? Inner classes are really ugly, so we turned to a switch:

void resolveAll() {
	a = resolve(State.A);
	b = resolve(State.B);
	c = resolve(State.C);
} 
	
@SuppressWarnings("unchecked")
private <T> T resolve(State state) {
	T resolved = null;
	switch (state) {
		case A:
			resolved = (T) resolveA();
			break;
		case B:
			resolved = (T) resolveB();
			break;
		case C:
			resolved = (T) resolveC();
			break;
	} 

	if (resolved == null) {
		states.remove(state);
	}  else {
		states.add(state);
	} 
	
	return resolved;
} 

Not that much simpler than the original code. However, we are moving slowly to Java 8. With lambdas, inner classes do not look so ugly anymore:

void resolveAll() {
	a = resolve(State.A, this::resolveA);
	b = resolve(State.B, this::resolveB);
	c = resolve(State.C, this::resolveC);
} 

private <T> T resolve(State state, Supplier<T> resolver) {
	T resolved = resolver.get();
	
	if (resolved == null) {
		states.remove(state);
	}  else {
		states.add(state);
	} 
	
	return resolved;
}