Slightly deceiving error message on Collectors.toMap

[Paul Sandoz]

On Apr 17, 2014, at 9:37 PM, Michael Nascimento <misterm at gmail.com> wrote:

> First of all, the bug:
> 
> https://bugs.openjdk.java.net/browse/JDK-8040892
> 

Thanks!


> On Thu, Apr 17, 2014 at 3:51 PM, Peter Levart <peter.levart at gmail.com> wrote:
>> Or, more generaly: "two elements of resulting stream, which I can't tell
>> you, mapped to the same key, which I can't tell you either, but they also
>> mapped to values V1 and V2", ...
> 
> Exactly...
> 
>> Original elements are lost by the time two maps get merged together, but the
>> duplicate key could be retrieved with a little trickery:
>> 
>> 
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Collectors.duplicateKey/webrev.01/
> 

Thanks Peter.

FWIW i would have used the same exception throwing trick :-)

Originally throwingMerger was public, but we hesitated about it's utility and made it private, glad we did that now.


> Hmmm, if Brian approves using this spare Exception subclass, I suggest
> extracting the creation of the accumulator into its own method.
> 

Rather than the MergedRefusedException overriding fillInStackTrace it may be better to defer to the super constructor:

  MergeRefusedException(Object u, Object v) {
     super(null, null, false, false)
     this.u = u;
     this.v = v;
  }

as that sets up the correct conditions for the state of the fields.

Also since this is only relevant for the to*Map functions when no merger is explicitly provided it is tempting to rejig the implementation of those to create a Collector and wrap the existing mapMerger function, thus limiting the scope of the exception to where it is used.

I have a slight concern that for large values the exception could hold large string values, but i don't think we try very hard generally in the JDK to use a form of pretty truncated printing...


Peter, i believe you have committer rights? Perhaps you could submit on core-libs for review?

Paul.