Hello! This is my first patch, so I apologize in advance if I've done anything incorrectly (including botching the formatting of the mercurial diff below).
I think it would be beneficial to have an overloaded `Map.computeIfAbsent` method where the computed value does not depend on the key; for that reason, I submitted [1]. By taking a `Supplier<? extends Value>`, this will allow users to utilize method references where applicable. For example, if they create a `Map<String, Collection<Integer>>`, then the following can be used when computing values: map.computeIfAbsent("key", ArrayList::new).add(1); I originally planned to add overloaded methods for `Map.computeIfPresent` and `Map.compute` as well, but it would make more sense for them to take a `Function` (instead of a `Supplier`) if the computed value does not depend on the key. If this patch is successful, then I'll go forward with that planned change. To clarify, an overloaded `Map.computeIfAbsent` method was the only change, along with the unit test. I used JMH to benchmark my implementation versus an inlined implementation (not calling the existing `Map.computeIfAbsent` with the result of the supplier), and the difference was negligible (maybe the compiler inlined it?). Please let me know if I should inline it so I can change my implementation. [1]: https://bugs.openjdk.java.net/browse/JDK-8202521 ------------------------------------------------------ # HG changeset patch # User jhg023 # Date 1525366367 25200 # Thu May 03 09:52:47 2018 -0700 # Node ID 395def2871e8d077b382d722efc59b38373327d1 # Parent ea246151be08995543d0c9281099608bc9207a19 8202521: Add overloaded methods of Map#compute, Map#computeIfAbsent, Map#computeIfPresent Summary: Added "Map.computeIfAbsent(K key, Supplier<? extends V> supplier)" Reviewed-by: N/A Contributed-by: Jacob Glickman <jhg...@bucknell.edu> diff -r ea246151be08 -r 395def2871e8 src/java.base/share/classes/java/util/Hashtable.java --- a/src/java.base/share/classes/java/util/Hashtable.java Wed May 02 11:21:27 2018 -0700 +++ b/src/java.base/share/classes/java/util/Hashtable.java Thu May 03 09:52:47 2018 -0700 @@ -29,6 +29,7 @@ import java.util.function.BiConsumer; import java.util.function.Function; import java.util.function.BiFunction; +import java.util.function.Supplier; import jdk.internal.misc.SharedSecrets; /** @@ -1054,6 +1055,21 @@ * {@inheritDoc} * * <p>This method will, on a best-effort basis, throw a + * {@link java.util.ConcurrentModificationException} if the + * supplier modified this map during computation. + * + * @throws ConcurrentModificationException if it is detected that the + * supplier modified this map + */ + @Override + public synchronized V computeIfAbsent(K key, Supplier<? extends V> supplier) { + return computeIfAbsent(key, k -> supplier.get()); + } + + /** + * {@inheritDoc} + * + * <p>This method will, on a best-effort basis, throw a * {@link java.util.ConcurrentModificationException} if the remapping * function modified this map during computation. * diff -r ea246151be08 -r 395def2871e8 src/java.base/share/classes/java/util/Map.java --- a/src/java.base/share/classes/java/util/Map.java Wed May 02 11:21:27 2018 -0700 +++ b/src/java.base/share/classes/java/util/Map.java Thu May 03 09:52:47 2018 -0700 @@ -28,6 +28,7 @@ import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.function.Supplier; import java.io.Serializable; /** @@ -1010,6 +1011,85 @@ } /** + * If the specified key is not already associated with a value (or is mapped + * to {@code null}), attempts to compute its value using the given supplier + * and enters it into this map unless {@code null}. + * + * <p>If the supplier returns {@code null}, no mapping is recorded. + * If the supplier itself throws an (unchecked) exception, the + * exception is rethrown, and no mapping is recorded. The most + * common usage is to construct a new object serving as an initial + * mapped value or memoized result that does not depend on the key, + * as in: + * + * <pre> {@code + * map.computeIfAbsent(key, () -> new Value()); + * }</pre> + * + * <p>Or to implement a multi-value map, {@code Map<K,Collection<V>>}, + * supporting multiple values per key: + * + * <pre> {@code + * map.computeIfAbsent(key, HashSet::new).add(v); + * }</pre> + * + * <p>The supplier should not modify this map during computation. + * + * @implSpec + * The default implementation is equivalent to the following steps for this + * {@code map}, then returning the current value or {@code null} if now + * absent: + * + * <pre> {@code + * if (map.get(key) == null) { + * V newValue = supplier.get(); + * if (newValue != null) + * map.put(key, newValue); + * } + * }</pre> + * + * <p>The default implementation makes no guarantees about detecting if the + * supplier modifies this map during computation and, if appropriate + * reporting an error. Non-concurrent implementations should + * override this method and, on a best-effort basis, throw a + * {@code ConcurrentModificationException} if it is detected that the + * supplier modifies this map during computation. Concurrent + * implementations should override this method and, on a best-effort basis, + * throw an {@code IllegalStateException} if it is detected that the + * supplier modifies this map during computation and as a result + * computation would never complete. + * + * <p>The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. In particular, all implementations of + * subinterface {@link java.util.concurrent.ConcurrentMap} must document + * whether the supplier is applied once atomically only if the value + * is not present. + * + * @param key key with which the specified value is to be associated + * @param supplier the supplier to compute a value + * @return the current (existing or computed) value associated with + * the specified key, or null if the computed value is null + * @throws NullPointerException if the specified key is null and + * this map does not support null keys, or the supplier + * is null + * @throws UnsupportedOperationException if the {@code put} operation + * is not supported by this map + * (<a href="{@docRoot}/java.base/java/util/Collection.html#optional-restrictions">optional</a>) + * @throws ClassCastException if the class of the specified key or value + * prevents it from being stored in this map + * (<a href="{@docRoot}/java.base/java/util/Collection.html#optional-restrictions">optional</a>) + * @throws IllegalArgumentException if some property of the specified key + * or value prevents it from being stored in this map + * (<a href="{@docRoot}/java.base/java/util/Collection.html#optional-restrictions">optional</a>) + * @since 11 + */ + default V computeIfAbsent(K key, Supplier<? extends V> supplier) { + return computeIfAbsent(key, k -> supplier.get()); + } + + /** * If the value for the specified key is present and non-null, attempts to * compute a new mapping given the key and its current mapped value. * diff -r ea246151be08 -r 395def2871e8 test/jdk/java/util/Map/Defaults.java --- a/test/jdk/java/util/Map/Defaults.java Wed May 02 11:21:27 2018 -0700 +++ b/test/jdk/java/util/Map/Defaults.java Thu May 03 09:52:47 2018 -0700 @@ -25,7 +25,7 @@ * @test * @bug 8010122 8004518 8024331 8024688 * @summary Test Map default methods - * @author Mike Duigou + * @author Mike Duigou, Jacob Glickman * @run testng Defaults */ @@ -301,6 +301,27 @@ assertSame(map.get(null), EXTRA_VALUE, "not expected value"); } + @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull") + public void testComputeIfAbsentNullsSupplier(String description, Map<IntegerEnum, String> map) { + // null -> null + assertTrue(map.containsKey(null), "null key absent"); + assertNull(map.get(null), "value not null"); + assertSame(map.computeIfAbsent(null, () -> null), null, "not expected result"); + assertTrue(map.containsKey(null), "null key absent"); + assertNull(map.get(null), "value not null"); + assertSame(map.computeIfAbsent(null, () -> EXTRA_VALUE), EXTRA_VALUE, "not mapped to result"); + // null -> EXTRA_VALUE + assertTrue(map.containsKey(null), "null key absent"); + assertSame(map.get(null), EXTRA_VALUE, "not expected value"); + assertSame(map.remove(null), EXTRA_VALUE, "removed unexpected value"); + // null -> <absent> + assertFalse(map.containsKey(null), "null key present"); + assertSame(map.computeIfAbsent(null, () -> EXTRA_VALUE), EXTRA_VALUE, "not mapped to result"); + // null -> EXTRA_VALUE + assertTrue(map.containsKey(null), "null key absent"); + assertSame(map.get(null), EXTRA_VALUE, "not expected value"); + } + @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all") public void testComputeIfAbsent(String description, Map<IntegerEnum, String> map) { // 1 -> 1 @@ -321,6 +342,25 @@ } @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all") + public void testComputeIfAbsentSupplier(String description, Map<IntegerEnum, String> map) { + // 1 -> 1 + assertTrue(map.containsKey(KEYS[1])); + Object expected = map.get(KEYS[1]); + assertTrue(null == expected || expected == VALUES[1], description + String.valueOf(expected)); + expected = (null == expected) ? EXTRA_VALUE : expected; + assertSame(map.computeIfAbsent(KEYS[1], () -> EXTRA_VALUE), expected, description); + assertSame(map.get(KEYS[1]), expected, description); + + // EXTRA_KEY -> <absent> + assertFalse(map.containsKey(EXTRA_KEY)); + assertNull(map.computeIfAbsent(EXTRA_KEY, () -> null)); + assertFalse(map.containsKey(EXTRA_KEY)); + assertSame(map.computeIfAbsent(EXTRA_KEY, () -> EXTRA_VALUE), EXTRA_VALUE); + // EXTRA_KEY -> EXTRA_VALUE + assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); + } + + @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=all values=all") public void testComputeIfAbsentNullFunction(String description, Map<IntegerEnum, String> map) { assertThrowsNPE(() -> map.computeIfAbsent(KEYS[1], null)); } @@ -370,7 +410,7 @@ assertThrowsNPE(() -> map.computeIfPresent(KEYS[1], null)); } - @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull") + @Test(dataProvider = "Map<IntegerEnum,String> rw=true keys=withNull values=withNull") public void testComputeNulls(String description, Map<IntegerEnum, String> map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null");