7

Why HashMap merge is doing null check on value. HashMap supports null key and null values.So can some one please tell why null check on merge is required?

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

Due to this I am unable to use Collectors.toMap(Function.identity(), this::get) to collect values in a Map

Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
Paul Erric
  • 466
  • 1
  • 5
  • 12
  • 1
    Did you consult the [documentation](https://docs.oracle.com/javase/9/docs/api/java/util/HashMap.html#merge-K-V-java.util.function.BiFunction-)? – Flown Feb 27 '18 at 06:50

3 Answers3

6

The behavior is mandated by the Map.merge contract:

Throws:

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

Note that using Map.merge for Collectors.toMap without a merge function is an implementation detail; it not only disallows null values, it does not provide the desired behavior for reporting duplicate keys, the Java 8 implementation wrongly reports one of the two values as key when there are duplicate keys.

In Java 9, the implementation has been completely rewritten, it does not use Map.merge anymore. But the new implementation is behavioral compatible, now having code explicitly throwing when the value is null. So the behavior of Collectors.toMap not accepting null values has been fixed in the code and is not an artifact of using Map.merge anymore. (Still speaking of the toMap collector without a merge function only.)

Unfortunately, the documentation does not tell.

Community
  • 1
  • 1
Holger
  • 285,553
  • 42
  • 434
  • 765
  • what do you mean that it does not use `Map#merge` anymore? I am looking at the code and actually seeing it... – Eugene Feb 27 '18 at 08:38
  • 1
    As said, *still speaking of the `toMap` collector without a merge function only*. Look for `uniqKeysMapAccumulator`. It uses `putIfAbsent`, after applying an explicit `null` check on the new value. – Holger Feb 27 '18 at 08:42
  • oh my! looked at the code, indeed what a hack with `putIfAbsent`, at least it would correctly report the key with the problem, thank you for the answer – Eugene Feb 27 '18 at 08:55
  • @Eugene well, it allows correct failure reports and still bears only one hash operation per element. What more can you get… – Holger Feb 27 '18 at 08:57
  • I don't know, it's just that every time I look into the sources I'm expecting super-human code – Eugene Feb 27 '18 at 08:58
3

Because internally for Collectors.toMap, Map#merge is used - you can't really do anything about it. Using the static Collectors.toMap is not an option (which by the way is documented to throw a NullPointerException).

But spinning a custom collector to be able to do what you want (which you have not shown) is not that complicated, here is an example:

 Map<Integer, Integer> result = Arrays.asList(null, 1, 2, 3)
            .stream()
            .collect(
                    HashMap::new,
                    (map, i) -> {
                        map.put(i, i);
                    },
                    HashMap::putAll);
Eugene
  • 117,005
  • 15
  • 201
  • 306
  • 6
    Unfortunately, `Collectors.toMap`’s documentation does not really say something about `NullPointerException`s. By the way, your lambda expression doesn’t really need the curly braces. – Holger Feb 27 '18 at 08:33
0

As a workaround for mentioned problems with null values in toMap and merge you can try to use a custom collector in the following manner:

public static <T, R> Map<T, R> mergeTwoMaps(final Map<T, R> map1,
                                            final Map<T, R> map2,
                                            final BinaryOperator<R> mergeFunction) {
    return Stream.of(map1, map2).flatMap(map -> map.entrySet().stream())
            .collect(HashMap::new,
                    (accumulator, entry) -> {
                        R value = accumulator.containsKey(entry.getKey()) 
                                ? mergeFunction.apply(accumulator.get(entry.getKey()), entry.getValue())
                                : entry.getValue();
                            accumulator.put(entry.getKey(), value);
                    },
                    HashMap::putAll);
}
fyrkov
  • 2,245
  • 16
  • 41