Useless null check in HashMap.merge()

[Jim Laskey]

Just as a note, it is reasonable to make value “final” so that value can not be nulled out between the test and its use.

Cheers,

— Jim


> On Jul 4, 2018, at 10:22 AM, Zheka Kozlov <orionllmain at gmail.com> wrote:
> 
> Oh yes, you are right. Then this is not dead code, just a useless null
> check.
> 
> 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.
>> 
>> 
>>