On Fri, 24 Mar 2023 22:16:54 GMT, Tagir F. Valeev <tval...@openjdk.org> wrote:
>> 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/ReverseOrderSortedMapView.java line 185:
>
>> 183:
>> 184: static <K, V> Iterator<K> descendingKeyIterator(SortedMap<K, V>
>> map) {
>> 185: return new Iterator<>() {
>
> Probably create `descendingEntryIterator` as base, and derive
> `descendingKeyIterator` and `descendingValueIterator`? This way, `map.get()`
> inside `descendingValueIterator` and `descendingEntryIterator` could be
> avoided.
Not sure this will help. The fundamentals of advancing the descending iterator
for SortedMap involve getting the last key of the current view (via the
`lastKey` method) and then excluding it from the view with `headMap(lastKey)`.
This allows iterating the keys in reverse without any `get` calls (though of
course the inclusive/exclusive searching for the last key does involve probing
the map). To build a descending entrySet iterator, one would have to do all of
the above and then add a `get` call to obtain the value to construct the Entry.
This adds a `get` call here but then saves a `get` call elsewhere. Seems like a
wash to me.
It's kind of a shame because `TreeMap` internally has `getLastEntry`. Its
`lastKey` method calls it and then throws away the Entry. :-( But this doesn't
matter, since `TreeMap` is a `NavigableMap` and its reverse iteration and views
are all based on `descendingMap`, so the `ReverseOrderSortedMapView` stuff
isn't used at all.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1172078247
