Hello, Please review the change for https://bugs.openjdk.java.net/browse/JDK-8238969:
http://cr.openjdk.java.net/~prappo/8238969/webrev.00/ During the last exploratory debugging session, I collected a number of issues that I think are worth fixing. These include: 1. Indentation, unnecessary (and hopefully non-controversial) parentheses, local variables' names, typos, method references (where practical), comments, etc. 2. Usages of the java.util.stream API. This could be thought of as complementary change to that [1] of by Jonathan Gibbons, though it stemmed from an unrelated investigation of mine. In a nutshell, some sites that use streams are order-sensitive, while others are not. Where the order is important, I used forEachOrdered() or changed the thing straight to Iterable.forEach(), eliding the in-between call to stream(). In some cases that allowed to simplify the thing into an atomic (as in "indivisible", not "concurrent-atomic") operation from Collections API. For example: - jdk/javadoc/internal/doclets/formats/html/LinkFactoryImpl.java:136,138 - jdk/javadoc/internal/doclets/toolkit/util/Utils.java:3268 Where the order was immaterial, I emphasized that by ensuring a non-deterministic semantics was used. For that reason, in some places I inserted an in-between call to stream(), if there was none previously. I'm still undecided on whether to use streams where the order is immaterial. It's not about optimization, but readability. I guess it all depends on the context and potential for using intermediate operations. It seems to be a no-brainer when no intermediate operations, such as filtering or mapping, are involved. Sadly, we don't seem to have good tools at hand for testing things like that. One way this change could be tested is through using "unfriendly" (work-to-rule) implementations of Collections and Stream APIs. For example, if the order of iteration is unspecified, or better still, explicitly specified to be non-deterministic, the implementation should make sure that the order is random. If a List is not of the RandomAccess flavor it should impose a perceivable penalty for getting elements by index, a call to Thread.sleep or a busy-loop would do. I'm sure such things exist in one form or another. We just don't have them, and implementing those on our own is a project not for the faint-hearted! Immutable collections, introduced in JEP 269, are good in this sense, though they are not enough: The iteration order of set elements/mappings is unspecified and is subject to change But I digress. The existing battery of tests passes. 3. Pinpoint uses of modern String APIs (and boy, did they fit in nicely!) - jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java:202 - jdk/javadoc/internal/tool/Start.java:289,291 4. Marking with in-line comments "Consider this while working on JDK-8238966" This is about future work on extracting joining-with-a-separator functionality. I believe those comments have value, even though the issue they mark is not addressed in this patch. Some of the sites seem to be a low-hanging fruit, but the point is to consider them as a whole before proceeding with a fix. 5. Removal of the Result.serialVersionUID field I couldn't think of any use for that field. My best guess is that it's just a leftover from the times when this enum was Error. Notes ===== a. jdk/javadoc/internal/doclets/formats/html/AbstractTreeWriter.java:143 Shouldn't it use equals() instead of `==` in this case? A quick look shows a surprising number of reference equality checks on javax.lang.model.element.Name and javax.lang.model.element.Element instances. Why would we need to use reference equality on types with explicitly defined equals() and hashCode()? b. What do people think of removing unused type members? Since jdk.javadoc doesn't seem to be using java.lang.reflect (beyond a couple of trivial interactions with Doclet SPI), the compile-time checking should be enough to make sure the removal is safe. The are some 100 (hundred) of unused methods across the jdk.javadoc codebase. -Pavel ------------------------------------------------------------------------------- [1] https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-February/001390.html
