Why does Collectors.toMap report value instead of key on Duplicate Key error?

Question

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;
        }
    }
}

Solution 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.

Solution 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…

Solution 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

Solution 4

In Java 8, duplicate values from two streams (or with same strems iterating twice) are not allowed to be merged into the Map at a time. You can use Collectors.groupingBy & Collectors.mapping to combine the results into the Map<Object, Set<Object>> or Map<Object, List<Object>> for processing.

    List<Employee> employees = new ArrayList<>();
    employees.add(new Employee("Sachin", 40));
    employees.add(new Employee("Sachin", 30));   
    employees.add(new Employee("Rahul", 30));

    Map<Integer, Set<String>> map = employees.stream()
                                    .collect(
                                            Collectors.groupingBy(
                                                                Employee::getAge, 
                                                                Collectors.mapping(
                                                                        Employee::getName,
                                                                        Collectors.toSet()
                                                                )
                                                            )
                                            );

Solution 5

Yes. It was fixed on Java 9. Message in Java 8.

Exception in thread "main" java.lang.IllegalStateException: Duplicate key HELLO IM NOT A KEY I AM A VALUE #1
    at java.util.stream.Collectors.lambda$throwingMerger$0(Collectors.java:133)
    at java.util.HashMap.merge(HashMap.java:1253)
    at java.util.stream.Collectors.lambda$toMap$58(Collectors.java:1320)
    at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    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 compass.issues.CollectorsToMapMessageIsWrong.main(CollectorsToMapMessageIsWrong.java:31)

And Java >=9 the message it's self explanatory.

For example in Java 15 i can get.

Exception in thread "main" java.lang.IllegalStateException: Duplicate key 1 (attempted merging values HELLO IM NOT A KEY I AM A VALUE #1 and HELLO IM NOT A KEY I AM A VALUE #2)
    at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:133)
    at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:180)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
    at java15.CollectorsToMapMessageIsWrong.main(CollectorsToMapMessageIsWrong.java:31)

View more solutions

Comments

[Aaron]

For the lazy incredulous, here's the code in ideone

[Alexis C.]

I think it's planned to change in Java 9 (see hg.openjdk.java.net/jdk9/jdk9/jdk/file/b877de2ea2f2/src/… - note that it's still in development).

[Holger]

This answer contains a work-around. It’s designed for the case where an explicit map supplier is intended, but it’s easy to substitute it with HashMap::new, which is equivalent to what the standard toMap does in Java 8 and Java 9.

[Holger]

Your collector still is subject to the original problem for parallel streams, as your merge function just uses the original implementation which will throw an error with the wrong message when there is a duplicate key encountered at this point. For a correct collector, you have to use the fixed function for both, accumulator and merge function, like in this answer, given two years ago.

[gil.fernandes]

@Holger Many thanks for pointing this out. I was indeed not aware of your answer which provides a better solution for the same problem. You are right that I my answer does not solve the problem for parallel streams.

[Tim Büthe]

Well, this is just an example of a merge function, which returns the new value. So later keys would replace former ones, like Map.put. But I think you should read the question again. It was more about the odd error message, which tuned out to be a bug in JDK which was fixed in later versions.

[Sushant Kumar Rout]

Yes, But the cause for this is user always expect what is the size of entry list that should be the size of extracted map. because as you know map always work with n->n mechanism.