Hi Stuart,
List.java:
- The varargs form would be more readable if the parameter was named
'elements' instead of 'es'.
- ditto Map and Set
Map.java:
- in the @returns I would say returns "a {@code Map}" to be explicit
about the return type.
(ditto in Set and List to reinforce the return type)
- the exception for duplicate keys that is thrown says duplicate
elements but
it is really the keys that are duplicated.
In the @throws clause it would be more readable to say "if there are
duplicates in the keys"
or as is appears in Set: "if there are any duplicate elements"
Reviewed: +1
Roger
On 12/03/2015 07:58 PM, Stuart Marks wrote:
Small refresh here: after some consultation with Brian Goetz and John
Rose, I've updated the class doc text covers immutability and
value-based. They now say,
* They are structurally immutable. Elements cannot be added, removed,
or replaced. Attempts to do so result in
UnsupportedOperationException. However, if the contained elements are
themselves mutable, this may cause the List's contents to appear to
change.
[and similar for Set and Map]
* They are value-based. Callers should make no assumptions about the
identity of the returned instances. Factories are free to create new
instances or reuse existing ones. Therefore, identity-sensitive
operations on these instances (reference equality (==), identity hash
code, and synchronization) are unreliable and should be avoided.
--
I still need an official OpenJDK Reviewer.
--
API:
http://cr.openjdk.java.net/~smarks/reviews/jep269/api.20151203/
Specdiff:
http://cr.openjdk.java.net/~smarks/reviews/jep269/specdiff.20151203/overview-summary.html
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/jep269/webrev.20151203/
Thanks,
s'marks
On 12/1/15 6:35 PM, Stuart Marks wrote:
Hi all,
Thanks for the previous round of review comments. Here's an updated
API and
implementation for review.
I've run specdiff with the --hu ("hide unchanged") option, so only
the bits of
the specification that have changed are shown. As before, though,
please ignore
the spurious change to EnumSet caused by a javadoc bug.
API changes:
- add clarifying notes on immutability
- remove wording that implied creation of new objects
- add "value-based" disclaimers
- add ordering specification for List and non-ordering disclaimers
for Set and Map
- clarify that Map.ofEntries() doesn't store the Map.Entry objects,
instead
it extracts keys and values
- Map.Entry instances returned from Map.entry() are *not* serializable
Other:
- markup cleanups
- small implementation cleanups
JEP:
http://openjdk.java.net/jeps/269
API spec (basically List, Map, and Set):
http://cr.openjdk.java.net/~smarks/reviews/jep269/api.20151201/
Specdiff:
http://cr.openjdk.java.net/~smarks/reviews/jep269/specdiff.20151201/overview-summary.html
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/jep269/webrev.20151201/
Thanks,
s'marks
