RFR: 8266571: Sequenced Collections [v8]

Nir Lisker nlisker at openjdk.org
Thu Apr 20 13:09:11 UTC 2023


On Thu, 20 Apr 2023 04:19:55 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> PR for Sequenced Collections implementation.
>
> Stuart Marks has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - Add missing @throws and @since tags.
>  - Convert code samples to snippets.
>  - Various editorial changes.
>  - Fix up toArray(T[]) on reverse-ordered views.

src/java.base/share/classes/java/util/NavigableMap.java line 442:

> 440:      * @return a reverse-ordered view of this map, as a {@code NavigableMap}
> 441:      * @since 21
> 442:      */

`NavigableSet` words its `reversed` doc a bit differently.

src/java.base/share/classes/java/util/SequencedMap.java line 127:

> 125:      * Returns a reverse-ordered <a href="Collection.html#view">view</a> of this map.
> 126:      * The encounter order of elements in the returned view is the inverse of the encounter
> 127:      * order of elements in this map. The reverse ordering affects all order-sensitive operations,

Although I think this is clear as-is, you defined *encounter order* in a map with respect to mappings, not elements, and that is also the language used in the methods here, so you might want to use this language here too.

src/java.base/share/classes/java/util/SequencedMap.java line 152:

> 150:         var it = entrySet().iterator();
> 151:         return it.hasNext() ? Map.Entry.copyOf(it.next()) : null;
> 152:     }

Would is be better to first check `Map.isEmpty()` and only then obtain the iterator? That would eliminate also the `hasNext` check.

default Map.Entry<K,V> firstEntry() {
    return isEmpty() ? null : Map.Entry.copyOf(entrySet().iterator().next());
}


Same question for the other methods.

src/java.base/share/classes/java/util/SequencedMap.java line 263:

> 261:      *
> 262:      * @implSpec
> 263:      * The implementation of this method in this class returns a {@code SequencedSet}

Some of the methods here use "of this method" and some skip it, noting in case you want to streamline it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1172518086
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1172493367
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1172499047
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1172507205


More information about the core-libs-dev mailing list