Hi Martin,
May I ask what do you mean by pasting this pice of code? Sorry I didn't get it.
Best,
John Lin
Martin Buchholz <marti...@google.com> 於 2020年6月12日 週五 下午2:42寫道：
>
> import java.util.HashMap;
> import java.util.Map;
> import java.util.function.BiFunction;
>
> @SuppressWarnings("serial")
> public class MapsComputeNull {
>
> static class HashMap1<K,V> extends HashMap<K,V> {
> @Override public V compute(
> K key,
> BiFunction<? super K, ? super V, ? extends V>
> remappingFunction) {
> final Map<K,V> map = this;
> V oldValue = map.get(key);
> V newValue = remappingFunction.apply(key, oldValue);
> if (newValue != null) {
> map.put(key, newValue);
> } else {
> map.remove(key);
> }
> return newValue;
> }
> }
>
> static class HashMap2<K,V> extends HashMap<K,V> {
> @Override public V compute(
> K key,
> BiFunction<? super K, ? super V, ? extends V>
> remappingFunction) {
> final Map<K,V> map = this;
> V oldValue = map.get(key);
> V newValue = remappingFunction.apply(key, oldValue);
> if (newValue != null) {
> map.put(key, newValue);
> } else if (oldValue != null) {
> map.remove(key);
> }
> return newValue;
> }
> }
>
> public static void main(String[] args) throws Throwable {
> test(new HashMap<Object, Object>());
> test(new HashMap1<Object, Object>());
> test(new HashMap2<Object, Object>());
> }
>
> static void test(Map<Object, Object> map) {
> Object key = 42;
> map.put(key, null);
> map.compute(key, (k, v) -> null);
> System.out.printf("%s %s%n",
> map.getClass().getSimpleName(),
> map.containsKey(key));
> }
> }
>
> On Thu, Jun 11, 2020 at 10:06 PM 林自均 <johnl...@gmail.com> wrote:
> >
> > 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 <marti...@google.com> 於 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 林自均 <johnl...@gmail.com> 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 <johnl...@gmail.com>
> > > > # 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
