On Wed, 25 Nov 2020 16:39:20 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> @pavelrappo Please see my proposed CSR below. Thank you.
>>
>> # Map::compute should have a clearer implementation requirement.
>>
>> ## Summary
>>
>> java.util.Map::compute should have a clearer implementation requirement in
>> its documentation.
>>
>> ## Problem
>>
>> The documentation of the implementation requirements for Map::compute has
>> the following problems:
>> 1. It lacks of return statements for most of the if-else cases.
>> 1. The indents are 3 spaces, while the convention is 4 spaces.
>> 1. The if-else is overly complicated and can be simplified.
>>
>> ## Solution
>>
>> Rewrite the documentation of Map::compute to match its default
>> implementation.
>>
>> ## Specification
>>
>> diff --git a/src/java.base/share/classes/java/util/Map.java
>> b/src/java.base/share/classes/java/util/Map.java
>> index b1de34b42a5..c3118a90581 100644
>> --- a/src/java.base/share/classes/java/util/Map.java
>> +++ b/src/java.base/share/classes/java/util/Map.java
>> @@ -1113,17 +1113,12 @@ public interface Map<K, V> {
>> * <pre> {@code
>> * V oldValue = map.get(key);
>> * V newValue = remappingFunction.apply(key, oldValue);
>> - * if (oldValue != null) {
>> - * if (newValue != null)
>> - * map.put(key, newValue);
>> - * else
>> - * map.remove(key);
>> - * } else {
>> - * if (newValue != null)
>> - * map.put(key, newValue);
>> - * else
>> - * return null;
>> + * if (newValue != null) {
>> + * map.put(key, newValue);
>> + * } else if (oldValue != null) {
>> + * map.remove(key);
>> * }
>> + * return newValue;
>> * }</pre>
>> *
>> * <p>The default implementation makes no guarantees about detecting if
>> the
>
> @johnlinp In any case, you'd also need to update the surrounding prose; we
> need to remove this:
> returning the current value or {@code null} if absent:```
@pavelrappo
> What is the required level of fidelity particular (pseudo-) code has to have?
It's potentially a large discussion, one that could be had in the context of my
JEP draft http://openjdk.java.net/jeps/8068562 . However, speaking practically,
it's possible to focus the discussion fairly concisely: the main responsibility
of the `@implSpec` ("Implementation Requirements") section is to give
implementors of subclasses enough information to decide whether to inherit the
implementation or to override it, and if they override it, what behavior they
can expect if they were to call `super.compute`.
In this case, a null-value-tolerating Map implementation needs to know that the
default implementation calls `remove` in the particular case that you
mentioned. A concurrent Map implementation will also need to know that the
default implementation calls `get(key)` and `containsKey(key)` at different
times, potentially leading to a race condition. Both of these inform the
override vs. inherit decision.
-------------
PR: https://git.openjdk.java.net/jdk/pull/714
