Map.merge Javadoc is confusing

[David Holmes]

On 22/11/2013 5:02 AM, Louis Wasserman wrote:
> While I agree that case should be specified, I'm not certain I follow why
> that's what's going on here.
>
> The weird condition is that if oldValue is null, not value; oldValue is the
> old result of map.get(key).  The Javadoc seems not just unspecified, but
> actively wrong.  Here's a worked example:
>
> Map<String, Integer> map = new HashMap<>();
> map.merge("foo", 3, Integer::plus);
> Integer fooValue = map.get("foo");
>
> Going through the Javadoc-specified default implementation:
>
>    V oldValue = map.get(key); // oldValue is null
>    V newValue = (oldValue == null) ? value :
>                 remappingFunction.apply(oldValue, value);
>       // newValue is set to value, which is 3
>    if (newValue == null) // newValue is nonnull, branch not taken
>        map.remove(key);
>    else if (oldValue == null) // oldValue is null, branch taken
>        map.remove(key); // removes the entry from the map
>    else // else if was already triggered, branch not taken
>        map.put(key, newValue);
>
> In short, according to the Javadoc, fooValue should be null.  This seems
> like it's not intended.

Ah I see your point. The actual implementation doesn't do that of 
course. So a couple of issues with this particular method.

David

>
> On Thu, Nov 21, 2013 at 10:53 AM, David Holmes <david.holmes at oracle.com>wrote:
>
>> On 22/11/2013 4:17 AM, Louis Wasserman wrote:
>>
>>> The Javadoc for
>>> Map.merge<http://download.java.net/jdk8/docs/api/java/
>>> util/Map.html#merge-K-V-java.util.function.BiFunction->states
>>>
>>> that its default implementation is equivalent to:
>>>
>>> V oldValue = map.get(key);
>>>    V newValue = (oldValue == null) ? value :
>>>                 remappingFunction.apply(oldValue, value);
>>>    if (newValue == null)
>>>        map.remove(key);
>>>    else if (oldValue == null)
>>>        map.remove(key);
>>>    else
>>>        map.put(key, newValue);
>>>
>>> I'm somewhat confused by the second branch of this if statement, which is
>>> reached if newValue is nonnull and oldValue is null.  Does this really
>>> *remove* the entry for that key?  I would have expected there to be only
>>>
>>> two branches: either remove the entry if newValue is null, or put newValue
>>> if it's nonnull.
>>>
>>
>> There seems to be a hole in the spec regarding how a null value parameter
>> is handled. The default implementation treats it as-if the
>> remappingFunction returns null. Not unreasonable but not specified.
>>
>> David
>>
>>
>
>