RFR: 8004518 & 8010122 : Default methods on Map (original) (raw)

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:

Why you use both, {@code...} and ... ?

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