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