On Mon, 4 Jul 2022 16:02:31 GMT, Andrey Turbanov <[email protected]> wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletManager.java
>>  line 314:
>> 
>>> 312:             return;
>>> 313:         }
>>> 314:         Taglet tag = allTaglets.remove(tagName);
>> 
>> The logic was somewhat clearer before. This code may be correct, but it 
>> would help to have an explanatory comment.
>> Note this code is only executed once per run of javadoc, so performance is 
>> definitely not an issue.
>
> Hm.  For me the previous code is confusing. Subsequent remove+put by the same 
> key looks like some leftovers after refactoring.
> Javadoc of this method is very explanatory. New code just repeats what is 
> written there.  Not sure if additional comment required.

In a context of a linked hash map, remove+put has the effect of reordering. In 
your refactor there's still a remove+put on the same key, you essentially just 
hoisted the two common occurrences from both branches to outside the if 
statement.

Not to _necessarily_ appeal to authority, but Jon is a true multi-decade 
veteran of Java language tooling and knows more about Javadoc than most of us 
combined, so if all he asks for is a comment, I'd add a comment :-). Maybe 
something to the effect of


// remove + put in both branches below move the tag to the back of the map's 
ordering

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

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

Reply via email to