RFR: 8004518 & 8010122 : Default methods on Map

Ulf Zibis Ulf.Zibis at CoSoCo.de
Tue Apr 9 00:08:51 UTC 2013


Hi Mike,

Comments for j.u.Map:

To my savour the variant belongs to the left hand side of a comparison, e.g.:
    if (v = get(key) != null)

Instead
  501         return (null != (v = get(key)))
  502             ? v
  503             : containsKey(key) ? null : defaultValue;
I would code
  501         return ((v = get(key) != null) || containsKey(key)) ? v : defaultValue;

Use consistent formatted code examples in javadoc. I like the style from lines 558 to 561, but e.g. 
from line 601 to 605:
- 2 spaces before <pre>
- indentation should be 4
- line break before }</pre>

Why you use both, {@code...} and <tt>...</tt> ?

For performance reasons, I think you should reverse the order of the if expressions here:
  673         if (!Objects.equals(get(key), value) || !containsKey(key))
... because otherwise map lookup too often becomes executed twice, via contains() + get().

Not negate the comparison term, e.g.:
1053                 if (value == null)
1054                     return null;
1055                 if ((oldValue = putIfAbsent(key, value)) == null)
1056                     return value;


-Ulf


Am 08.04.2013 20:07, schrieb Mike Duigou:
> Hello all;
>
> This is a combined review for the new default methods on the java.util.Map interface being added for the JSR-335 lambda libraries. The reviews are being combined because they share a common unit test.
>
> http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/
>
> 8004518: Add in-place operations to Map
>   forEach()
>   replaceAll()
>
> 8010122: Add atomic operations to Map
>   getOrDefault()
>   putIfAbsent()          *
>   remove(K, V)
>   replace(K, V)
>   replace(K, V, V)
>   compute()              *
>   merge()                *
>   computeIfAbsent()      *
>   computeIfPresent()     *
>
> The * operations treat null values as being absent. (ie. the same as there being no mapping for the specified key).
>
> The default implementations provided in Map are overridden in HashMap for performance purposes, in Hashtable for atomicity and performance purposes and in Collections for atomicity.
>
> Mike
>   




More information about the core-libs-dev mailing list