RFR: 8284447: Remove the unused NestedClassWriter interface [v2]

[Jonathan Gibbons]

On Wed, 6 Apr 2022 22:49:03 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> The `NestedClassWriter` interface seems to have never been used. I delete it and reuse the freed name for the `NestedClassWriterImpl` class. If I don't rename `NestedClassWriterImpl` to `NestedClassWriter`, it might confuse future maintainers. Indeed, the "Impl" suffix on a class' name typically implies that there is an interface which that class implements.
>
> Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge branch 'master' into 8284447
>  - Initial commit

Sigh, I "reviewed" this earlier, but apparently did not complete the review. So I'll try and reconstruct my earlier comments.

Approving this change is not as simple as it may seem. Yes, `NestedClassWriter` is unused, and while removing it may seem to be the right thing to do, renaming the `Impl` class just swaps one weirdness for another ... In particular, what was the `Impl` class is now stylistically inconsistent with other related/similar subtypes of `AbstractMemberWriter`, and that shows up in the first few lines of the review (in `ClassUseWriter`).    As curious/weird as the world was before this PR, it is still somewhat weird after this PR, but just in a new and different way.

The underlying problem is that we have no single good naming methodology for interfaces and implementations across the `toolkit` world and the `formats.html` world.  In particular, in `formats.html` we inconsistently have interfaces and implementations, `Xyz` interfaces and classes, and `XyzImpl` classes and `HtmlXyz` classes.

For what it's worth, I think a better solution from a consistency point of view would be to not delete `NestedClassWriter`, and have `NestedClassWriterImpl` implement `NestedClassWriter`, even though it is (currently) an empty interface, and even though it may be likely to remain so.

That being said, I'll (reluctantly?) approve this as-is, since I think there is a bigger problem to be addressed, which is the overall general "Writer" architecture in terms of the inheritance hierarchy and packages used to model writers (and that is even before we look at the similar but different "Builder" architecture.)  Overall, I don't think this change makes the world any worse, just annoyingly different, and it is arguably a tiny bit better for removing an unused empty interface.

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

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8129