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
