[PATCH] 8247402: Rewrite the implementation requirements for Map::compute()

[林自均]

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 <martinrb at 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 林自均 <johnlinp at 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 <johnlinp at 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