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.hol...@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
