On Sat, 22 Apr 2023 07:56:03 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> Might not work if this map is concurrent. If isEmpty() returns false but >> then the map is emptied before the subsequent calls, next() might throw >> NSEE. The concurrent maps' iterators make sure to cache the element if >> hasNext() has returned true. > > Then shouldn't `ConcurrentNavigableMap`, the only `ConcurrentMap` and > `SequencedMap` supertype (there is no `ConcurrentSortedMap`), override its > superinterface implementation with this thread-safe one? > > I think that the question boils down to: should `SequencedMap` take into > account the needs of its subtypes and give a conservative default > implementation, or should it give a "minimal" implementation and let the > subtypes override it according to their needs? I would choose the latter, > both because `SequencedMap` specifies no guarantees on thread-safety > (atomicity and synchronization), and because `Map`'s default implementation > behave this way - `compute`, `merge`... tell concurrent implementations how > to override them. > > Now if someone implements a `SequencedMap` that is also a `ConcurrentMap`, > but is not a `NavigableMap`, say `ConcurrentLinkedHashMap`, they will > override the `SequencedMap` defaults like they would the `Map` defaults. If somebody is implementing a concurrent or thread-safe SequencedMap, they might look at the spec of the default method and determine that it's safe to inherit the default implementation (if, as I mentioned above, the map's Iterator has the right properties). Changing the default implementation to test isEmpty() first _requires_ overriding in to be thread-safe, introducing the possibility of a new kind of error. I don't see any utility in changing this default implementation or adding an overriding default implementation to another one of the interfaces. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1175907436
