RFR: 8266571: Sequenced Collections [v7]

Stuart Marks smarks at openjdk.org
Wed Apr 19 18:01:28 UTC 2023


On Wed, 19 Apr 2023 05:51:11 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Stuart Marks has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Remove unnecessary 'final' from a couple places.
>>  - Clarify ordering of Collection.addAll and Map.putAll; add links to
>>    encounter order.
>>  - Make constructors private for several reverse-ordered views.
>
> src/java.base/share/classes/java/util/Collection.java line 511:
> 
>> 509:      * nonempty.) If the specified collection has a defined
>> 510:      * <a href="SequencedCollection.html#encounter">encounter order</a>,
>> 511:      * processing of its elements generally occurs in that order.
> 
> Shouldn't `remove` also contain such a note? From its docs:
> 
> "More formally, removes an element e such that `Objects.equals(o, e)`, if this collection contains one or more such elements." Can add something like "If the specified collection has a defined *encounter order*, generally removes the first encountered element in that order".

No, the stipulation for removing the first element only applies to explicit-positioned collections like List and Deque.

> src/java.base/share/classes/java/util/Collections.java line 374:
> 
>> 372:      * @apiNote
>> 373:      * This method mutates the specified list in-place. To obtain a
>> 374:      * reversed-ordered view of a list without mutating it, use the
> 
> The term that has been usually used is "reverse-ordered".

Typo.

> src/java.base/share/classes/java/util/Collections.java line 380:
> 
>> 378:      * @throws UnsupportedOperationException if the specified list or
>> 379:      *         its list-iterator does not support the {@code set} operation.
>> 380:      * @see    List#reversed List.reversed
> 
> Is the `@see` tag really useful if you're already linking the method directly?

Requested by another reviewer.

> src/java.base/share/classes/java/util/SequencedCollection.java line 34:
> 
>> 32:  * from the first element to the last element. Given any two elements, one element is
>> 33:  * either before (closer to the first element) or after (closer to the last element)
>> 34:  * the other element.
> 
> The last sentence seems a bit clumsy owing to the abundance of parenthesized content and the confusion of "first element" of the collection with the first element of the 2 chosen for the explanation. How about something like:
> "Given any two elements, one element is either before the other element (it is closer to the collection's start) or after it (closer to the collection's end)"?

Doesn't seem like an improvement.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1171686221
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1171688269
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1171686536
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1171687102


More information about the core-libs-dev mailing list