Re: RFR: 8029055: Map.merge must refuse null values

2013-12-07 Thread Doug Lea
On 12/06/2013 05:18 PM, Mike Duigou wrote: Hello all. I have updated the webrev http://cr.openjdk.java.net/~mduigou/JDK-8029055/2/webrev/ In light of all the churn on merge and related methods, I did an editorial pass on jsr166 version of ConcurrentMap. I added a couple of paragraphs at

Re: Poor Javadoc of Map default methods [Re: RFR: 8029055: Map.merge must refuse null values]

2013-12-06 Thread Stephen Colebourne
See https://bugs.openjdk.java.net/browse/JDK-8029676 Stephen On 26 November 2013 13:20, Stephen Colebourne scolebou...@joda.org wrote: I took a quick look, but jumped back in horror at the start of the Javadoc for the new methods in Map. A Javadoc description should start with the positive,

Re: RFR: 8029055: Map.merge must refuse null values

2013-12-06 Thread Mike Duigou
Hello all. I have updated the webrev with a final correction from Brian Goetz, the @throws NPE didn't reflect that it was thrown unconditionally if value is null. http://cr.openjdk.java.net/~mduigou/JDK-8029055/2/webrev/ Mike On Nov 25 2013, at 20:32 , Mike Duigou mike.dui...@oracle.com

Re: RFR: 8029055: Map.merge must refuse null values

2013-12-05 Thread Brian Goetz
Coming late to the party... Overall I'll +1 this change, with the following nit: @throws NullPointerException if the specified key or value is null and * this map does not support null keys or values, or the * remappingFunction is null This should be: @throws

Re: RFR: 8029055: Map.merge must refuse null values

2013-12-03 Thread Mike Duigou
On Nov 26 2013, at 05:35 , Stephen Colebourne scolebou...@joda.org wrote: See the new thread for the general Javadoc issues. I'll focus on null here. Given a map {Key - null} I find the notion that putIfAbsent() or computeIfAbsent() ignore the existing mapping just plain wrong. While I can

Re: Poor Javadoc of Map default methods [Re: RFR: 8029055: Map.merge must refuse null values]

2013-11-27 Thread Stephen Colebourne
On 26 November 2013 17:35, Martin Buchholz marti...@google.com wrote: I haven't looked in depth, but I agree with Stephen's analysis. This API and its javadoc needs work. E.g. It's not clear that the purpose of Map.compute is to *update* the mapping for key in the map. I actually felt that

Poor Javadoc of Map default methods [Re: RFR: 8029055: Map.merge must refuse null values]

2013-11-26 Thread Stephen Colebourne
I took a quick look, but jumped back in horror at the start of the Javadoc for the new methods in Map. A Javadoc description should start with the positive, not the negative. I had to read the code to figure out what they are supposed to do. Here are the four poor Javadocs and some proposed

Re: RFR: 8029055: Map.merge must refuse null values

2013-11-26 Thread Stephen Colebourne
See the new thread for the general Javadoc issues. I'll focus on null here. Given a map {Key - null} I find the notion that putIfAbsent() or computeIfAbsent() ignore the existing mapping just plain wrong. While I can rationalise it (just) it is a horrible reworking of the meaning of absent.

Re: RFR: 8029055: Map.merge must refuse null values

2013-11-25 Thread Mike Duigou
On Nov 24 2013, at 16:31 , David Holmes david.hol...@oracle.com wrote: Hi Mike, There is still no clear spec for what should happen if the param value is null. I feel very uncomfortable the status quo of with null being ignored, used for a sentinel and also as value. The relations