On Apr 23, 2014, at 11:06 AM, Peter Levart <[email protected]> wrote:
> Hi, > > I propose a patch for: > > https://bugs.openjdk.java.net/browse/JDK-8040892 > +1, nice use of putIfAbsent. Just double checking, did you run the stream unit tests? I think key step was to separate map merge and accumulate functions for the toMap* methods not accepting a value merge function. Now that is the case one could still use merge, but that would be less efficient than putIfAbsent since that does not require a lambda capturing the key. That fix caused me to think that we should update the value merge function accepting toMap* methods for the case when the merge function returns null, as removing an entry would be most undesirable. A simple fix would be to throw an NPE if the merge function returns null, if it is acceptable to induce a side-effect of a removal, which seems reasonable given a partial result would any be returned if a null check is performed on the result of the merge function, otherwise that merge function requires wrapping e.g. mergeFunction.andThen(Objects::requireNonNull). Paul. > Here's the webrev: > > http://cr.openjdk.java.net/~plevart/jdk9-dev/Collectors.duplicateKey/webrev.02/ > > There has already been a discussion on the lambda-dev about this bug: > > http://mail.openjdk.java.net/pipermail/lambda-dev/2014-April/012005.html > > > Initially a fix has been proposed which used a private RuntimeException > subclass thrown by the merger function in order to transport the values that > attempted to be merged out of Map.merge() method to an outer context where > they could be combined with the key to produce a friendlier "duplicate key" > message. It turns out that there is an alternative approach that doesn't > require exception acrobatics - using Map.putIfAbsent() instead of > Map.merge(). I checked both methods in Map defaults and in HashMap > implementations for possible semantic differences. Same behaviour can be > achieved by requiring that the value passed to Map.putIfAbsent(key, value) is > non-null and throwing NPE if it isn't. > > > Regards, Peter
