On Thu, 20 Apr 2023 04:19:55 GMT, Stuart Marks <sma...@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
