Useless null check in HashMap.merge()

[Doug Lea]

On 07/04/2018 09:22 AM, Zheka Kozlov wrote:
> Oh yes, you are right. Then this is not dead code, just a useless null
> check.

My vague recollection is that the check was there to ensure that the
block was identical to those in the other methods when considering ways
to factor it out (none of which worked out, so it still sprawls). It
could be removed.

-Doug


> 
> 2018-07-04 19:55 GMT+07:00 Scott Palmer <swpalmer at gmail.com>:
> 
>> On Jul 4, 2018, at 5:42 AM, Zheka Kozlov <orionllmain at gmail.com> wrote:
>>>
>>> I noticed dead code in java.util.HashMap.merge():
>>>
>>> public V merge(K key, V value,
>>>                   BiFunction<? super V, ? super V, ? extends V>
>>> remappingFunction) {
>>>    if (value == null)
>>>        throw new NullPointerException();
>>>
>>>    ...
>>>
>>>    if (value != null) { *// Condition ' value != null' is always true*
>>>        if (t != null)
>>>            t.putTreeVal(this, tab, hash, key, value);
>>>        else {
>>>            tab[i] = newNode(hash, key, value, first);
>>>            if (binCount >= TREEIFY_THRESHOLD - 1)
>>>                treeifyBin(tab, hash);
>>>        }
>>>        ++modCount;
>>>        ++size;
>>>        afterNodeInsertion(true);
>>>    }
>>>    return value;
>>> }
>>>
>>> The code in the if branch will never be executed because `value` was
>>> previously checked at the beginning of the method.
>>>
>>> Is this a mistake?
>>
>> You mean it will ALWAYS be executed.  Yes, it looks to me like the ‘if’ is
>> useless.
>>
>>
>>
>