On Tue, 13 Dec 2022 04:36:24 GMT, Jaikiran Pai <j...@openjdk.org> 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

Reply via email to