On Wed, 19 Apr 2023 05:51:11 GMT, Nir Lisker <nlis...@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

Reply via email to