RFR: 8307840: SequencedMap view method specification and implementation adjustments

Stuart Marks smarks at openjdk.org
Tue Jun 6 16:49:39 UTC 2023


On Tue, 6 Jun 2023 04:58:46 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Adjust the specification of the `SequencedMap` sequenced-view methods, and adjust implementations to match.
>
> src/java.base/share/classes/java/util/AbstractMap.java line 908:
> 
>> 906:         public boolean add(E t) { throw uoe(); }
>> 907:         public boolean addAll(Collection<? extends E> c) { throw uoe(); }
>> 908:         public void clear() { view().clear(); }
> 
> Each of these methods should perhaps have the `@Override` to make it clear that these correspond to the interface this class is implementing. But I see that the `AbstractMap` class itself doesn't use `@Override` anywhere, so I think it's OK to keep it consistent in its current form in this PR.

Yeah the collections classes are inconsistent about using `@Override`. Some maintainers (Martin Buchholz, Doug Lea) prefer not to use it at all. However, some usage has crept in from time to time. Personally I tend not to use it, as I think it adds a lot of clutter and doesn't provide much value. The main value this annotation provides detecting an incorrectly declared override (e.g., declaring an equals(Foo) method instead of equals(Object) method). In practice this doesn't seem to happen. These skeletons of these internal collection classes are usually auto-generated by an IDE, which won't make this mistake. The collections are usually pretty well tested so if something isn't overridden properly it will usually be caught.

I'm considering ripping out all occurrences of `@Override` from collections and declaring the prevailing style not to use it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14267#discussion_r1219984529


More information about the core-libs-dev mailing list