RFR: 8300517: Refactor VisibleMemberTable (method members) [v2]

[Jonathan Gibbons]

On Fri, 10 Mar 2023 16:12:00 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java line 302:
>> 
>>> 300:         // TODO: consider possible ambiguities: multiple overridden methods
>>> 301:         ensureInitialized();
>>> 302:         assert !overriddenMethodTable.containsKey(null);
>> 
>> can we somehow do the assert at the time of `put` ?
>
> We sure can, but that might complicate the patch even more.
> 
> The beauty of assertions is that you can make them anywhere you like. Here, it nicely pairs with the `get` method suggesting that `map.get(x) == null` is the same as `map.containsKey(x)`.
> 
> Also, please consider this commit as a big but still intermediate step: it sprinkles code with assertions to better capture what's going on (as far as we/I understand it), but eventually most structures, including `overriddenMethodTable`, will be replaced.

Understood, but this is more about the general pattern for handling (any) bad values in (any) collection.

Yes, you can check the values when you retrieve them, but then all you know is that _someone_ added/put a bad value, but not _who_ or _when_. In general, it is better to detect that bad value as early as possible so that you still have the surrounding context of _who_ and _when_ to help determine _why_.

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

PR: https://git.openjdk.org/jdk/pull/12887