.. are not necessarily stored in the Map ...
On 12/4/2015 4:50 AM, Stephen Colebourne wrote:
Sorry for my delayed review. Basically, I'm happy with this. Some oddments: Map.ofEntries(Entry...) "The entry objects themselves are not stored in the map." This seems wrong. `Entry` might be implemented as a value, not an object in the future. And if so, it might form part of the state of an optimised list implementation. The use of <K,V> looks like it should have an extra space after the comma to me. I prefer to avoid apostrophes in docs "the map's key type" -> "the key type of the map" "the first mapping's key" -> "the key of the first mapping" I note that java.util.Stream uses empty() but this API uses of() for the empty case. I can see both points of view and don't have much of a preference. Is consistency with Stream needed? KeyValueHolder needs a <p> at line 41 thanks Stephen On 4 December 2015 at 00:58, Stuart Marks <stuart.ma...@oracle.com> 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
