RFR(m): update 2: JEP 269 initial API and skeleton implementation (JDK-8139232)
Stephen Colebourne
scolebourne at joda.org
Fri Dec 4 09:50:47 UTC 2015
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.marks at 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
More information about the core-libs-dev
mailing list