RFR(m): JEP 269 initial API and skeleton implementation (JDK-8139232)
Remi Forax
forax at univ-mlv.fr
Tue Nov 24 15:03:30 UTC 2015
Hi Stuart,
The other way to detect collision instead of checking the size if to use the return value of put, it should be null if it's a new key,
but the code is maybe less readable.
There are several instance of
@SafeVarargs
@SuppressWarnings("varargs")
if you use @SafeVarargs you should not to use @SuppressWarnings("varargs").
In Map.ofEntries, calling Objects.requireNonNull on the entry inside the for loop is not necessary given getKey is called on that entry.
For the Set, i think i prefer to avoid the call to Array.asList and use add instead,
HashSet<E> set = new HashSet<>(3);
set.add(Objects.requireNonNull(e1));
set.add(Objects.requireNonNull(e2)));
if (set.size() != 2) {
throw new IllegalArgumentException("duplicate elements");
}
return Collections.unmodifiableSet(set);
and Set.of(E...) can be simply written like this:
@SafeVarargs
static <E> Set<E> of(E... es) {
HashSet<E> set = new HashSet<>((int) (c.size()/.75f));
for (E e : es) { // throws NPE if es is null
Objects.requireNonNull(e);
set.add(e);
}
if (set.size() != es.length) {
throw new IllegalArgumentException("duplicate elements");
}
return Collections.unmodifiableSet(set);
}
For KeyValueHolder, in equals, you don't need to access to the getter here,
return key.equals(e.key) && value.equals(e.value);
and if you can fix something that baffle me with the usual implementations of Map.Entry,
a good hashCode for KeyValueHolder should have the property that hashCode for entry(a, b) is different form the hashCode of entry(b, a),
something like:
return key.hashCode() ^ Integer.rotateLeft(value.hashCode(), 16);
or any variations.
cheers,
Rémi
----- Mail original -----
> De: "Stuart Marks" <stuart.marks at oracle.com>
> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mardi 24 Novembre 2015 07:26:27
> Objet: RFR(m): JEP 269 initial API and skeleton implementation (JDK-8139232)
>
> Hi all,
>
> Please review these API and implementation changes that comprise the first
> integration of JEP 269. I intend to push this under the "initial API and
> skeleton implementation" subtask, JDK-8139232.
>
> Changes since the previous review:
> - more precise wording regarding static factory methods (thanks Remi)
> - add defensive copy and test for List.of(E...) (thanks Michael Hixson)
> - more null checking and tests
> - various small wording cleanups
> - @since 9
>
> I've left the fixed-arg overloads at ten for all three interfaces. The
> fixed-arg
> methods provide a useful fast path for initialization and non-desktop,
> non-server cases. The cost is some API clutter; ten elements or pairs is
> rather
> a lot. This number should be sufficient, though, to handle the vast majority
> of
> cases without having to switch to varargs. Note that the clutter is only in
> the
> API definitions, and it doesn't intrude into the point of call (at least for
> List and Set). For the Map case, it's particularly useful to have lots of
> fixed-arg overloads, since the varargs case requires switching APIs and
> adding
> more boilerplate.
>
> I've also updated the JEP text to reflect the current proposal, mainly the
> removal of the static factory methods from the concrete classes.
>
> JEP:
>
> http://openjdk.java.net/jeps/269
>
> API spec (basically List, Map, and Set):
>
> http://cr.openjdk.java.net/~smarks/reviews/jep269/api.20151123/
>
> Specdiff:
>
> http://cr.openjdk.java.net/~smarks/reviews/jep269/specdiff.20151123/overview-summary.html
>
> Webrev:
>
> http://cr.openjdk.java.net/~smarks/reviews/jep269/webrev.20151123/
>
> Thanks,
>
> s'marks
>
More information about the core-libs-dev
mailing list