On Sat, 21 Nov 2020 09:21:06 GMT, John Lin <github.com+1290376+johnl...@openjdk.org> wrote:
>> @johnlinp, you cannot create a CSR by yourself at the moment. Someone else >> will have to do that for you. Might as well be me. So here's my proposal: >> come up with the meat, then I'll help you with the paperwork. >> >> For starters, have a look at existing CSRs (you don't need a JBS account for >> that). For example, >> https://bugs.openjdk.java.net/issues/?jql=issuetype%3DCSR%20and%20Subcomponent%3Djava.util%3Acollections%20 >> >> Fill in an informal CSR template inline in this thread, and we'll proceed >> from that point. Is that okay? > > @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 The proposed CSR has a few problems that we need to resolve. 1. The **Specification** pseudo-code behaves differently from both the old pseudo-code and the actual implementation when `newValue == null && oldValue == null` and `map.containsKey(key) == true`. 2. The content of the **Solution** section seems irrelevant: aside from a couple of missing `return` statements the current pseudo-code is fine. We are after something else, aren't we? The bottom line is we should state the solution more clearly. 3. The **Summary** section differs from that of the JDK-8247402. ------------- PR: https://git.openjdk.java.net/jdk/pull/714
