Hi Martin, Thanks for the review! However, the corner case you mentioned seems to be related to the design choice of Map::compute(), and it's out of the scope of this change. What do you think?
Best, John Lin Martin Buchholz <[email protected]> 於 2020年6月12日 週五 上午11:42寫道： > > 林自均, pleased to meet you! > > This rewrite seems clear and clean. > > Would it be even clearer if we did > > if (newValue != null) { > map.put(key, newValue); > } else { > map.remove(key); > } > > Hmmm, what about the corner case where the map permits null values and > the old value is null? > > On Thu, Jun 11, 2020 at 7:45 PM 林自均 <[email protected]> wrote: > > > > Hi all, > > > > This is my first time contribution. Please let me know if I did > > something wrong. Thanks. Regards > > > > I'm working on this bug: https://bugs.openjdk.java.net/browse/JDK-8247402 > > > > The original problem is that the implementation requirements for > > Map::compute() lacks of return statements for some cases. However, I > > found that there are some other problems in it: > > > > 1. The indents are 3 spaces, while most of the indents are 4 spaces. > > 2. The if-else is overly complicated and can be simplified. > > > > My proposed patch that generated by `hg export -g` is: > > > > # HG changeset patch > > # User John Lin <[email protected]> > > # Date 1591923561 -28800 > > # Fri Jun 12 08:59:21 2020 +0800 > > # Node ID 03c9b5c9e632a0d6e33a1f13c98bb3b31b1bf659 > > # Parent 49a68abdb0ba68351db0f140ddac793b1c391bd5 > > 8247402: Rewrite the implementation requirements for Map::compute() > > > > diff --git a/src/java.base/share/classes/java/util/Map.java > > b/src/java.base/share/classes/java/util/Map.java > > --- 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 > > > > Best, > > John Lin
