On Tue, 13 Dec 2022 04:36:24 GMT, Jaikiran Pai <[email protected]> wrote:
>> Naoto Sato has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains four additional
>> commits since the last revision:
>>
>> - Making fields final, other clean-ups
>> - Merge branch 'master' into JDK-8298416-SealedConsole
>> - tidying up
>> - sealed Console, impl/if separation
>
> src/java.base/share/classes/java/io/Console.java line 108:
>
>> 106: public PrintWriter writer() {
>> 107: throw new UnsupportedOperationException(
>> 108: "Console class itself does not provide implementation");
>
> Hello Naoto, should we perhaps then mark this method (and thus the class too)
> as `abstract` and leave the sub-classes to provide this method's
> implementation? Same with the other methods where we now throw
> `UnsupportedOperationException`.
Thanks for the review, Jai. You're right that making it `abstract` would
eliminate implementations in `Console` class, but I was hesitant to do so, as
it is a spec change that would suggest readers to think that `Console` is
generally pluggable (which is not).
> src/java.base/share/classes/java/io/ConsoleImpl.java line 198:
>
>> 196: private Writer out;
>> 197: private PrintWriter pw;
>> 198: private Formatter formatter;
>
> It looks like we can mark `readLock`, `writeLock`, `reader`, `out`, `pw` and
> `formatter` as `final`.
Made them `final` along with other fix-ups.
-------------
PR: https://git.openjdk.org/jdk/pull/11615
