Why does Collectors.toMap report value instead of

2019-01-09 14:55发布

问题:

This is really a question about a minor detail, but I'm under the impression to get something wrong here. If you add duplicate keys using Collectors.toMap-method it throws an Exception with message "duplicate key ". Why is the value reported and not the key? Or even both? This is really misleading, isn't it?

Here's a little test to demonstrate the behaviour:

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

public class TestToMap {

    public static void main(String[] args) {

        List<Something> list = Arrays.asList(
            new Something("key1", "value1"),
            new Something("key2", "value2"),
            new Something("key3", "value3a"),
            new Something("key3", "value3b"));

        Map<String, String> map = list.stream().collect(Collectors.toMap(o -> o.key, o -> o.value));
        System.out.println(map);
    }

    private static class Something {
        final String key, value;

        Something(final String key, final String value){
            this.key = key;
            this.value = value;
        }
    }
}

回答1:

This is reported as a bug, see JDK-8040892, and it is fixed in Java 9. Reading the commit fixing this, the new exception message will be

String.format("Duplicate key %s (attempted merging values %s and %s)", k, u, v)

where k is the duplicate key and u and v are the two conflicting values mapped to the same key.



回答2:

As the other answers already state, it’s a bug that will be fixed in Java 9. The reason, why the bug arose, is, that toMap relies on Map.merge which has the signature

V merge(K key, V value, BiFunction<? super V,? super V,? extends V> remappingFunction)

This method will insert the key-value mapping, if there was no previous mapping for the key, otherwise, the remappingFunction will be evaluated to calculate the new value. So, if duplicate keys are not allowed, it’s straight-forward to provide a remappingFunction which will unconditionally throw an exception, and you’re done. But…if you look at the function signature, you’ll notice that this function only receives the two values to be merged, not the key.

When the throwingMerger was implemented for Java 8, it was overlooked that the first argument is not the key, but even worse, it is not straight-forward to fix.

You’ll notice, if you try to provide an alternative merger using the overloaded toMap collector. The key value simply isn’t in scope at this point. The Java 9 developers had to change the whole toMap implementation for the no-duplicate case to be able to provide an exception message reporting the affected key…



回答3:

This is a know bug in Jdk 8. The message thrown should either display at least two values for which there is a key collision or ideally the key for which the collision happened. Attached is the link for the same



回答4:

If you have this problem and are forced to use Java 8 in your project, then you will have to create your own collector to get a meaningful error message.

Below you can find an example of a collector which gives you a meaningful message similar to the one you would get with Java 9:

private static class Something {
    private final String k;
    private final String v;

    public Something(String k, String v) {
        this.k = k;
        this.v = v;
    }
}

public static void main(String[] args) throws IOException, ParseException {
    List<Something> list = Arrays.asList(
            new Something("key1", "value1"),
            new Something("key2", "value2"),
            new Something("key3", "value3a"),
            new Something("key3", "value3b"));

    Collector<Something, ?, Map<String, String>> eloquentCollector =
            Collector.of(HashMap::new, (map, something) -> putUnique(map, something.k, something.v),
                    (m1, m2) -> { m2.forEach((k, v) -> putUnique(m1, k, v)); return m1; });

    Map<String, String> map = list.stream().collect(eloquentCollector);
    System.out.println(map);
}

private static <K,V> void putUnique(Map<K,V> map, K key, V v1){
    V v2 = map.putIfAbsent(key, v1);
    if(v2 != null) throw new IllegalStateException(
            String.format("Duplicate key '%s' (attempted merging incoming value '%s' with existing '%s')", key, v1, v2));
}

If you run this code, you will see the following error message pointing to the duplicate key and merge values:

Duplicate key 'key3' (attempted merging values value3a and value3b)

You should get the same exception message if you use in the example above parallelStream(), like e.g:

Map<String, String> map = list.parallelStream().collect(eloquentCollector);

Credits to Holger for pointing out that the original answer was not OK for parallel streams.