On Tue, 20 Dec 2022 14:44:38 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Actually, I see that it isn't possible for these to be final because we set
>> these up in the `console(...)` instance method of this class. That's because
>> this `JdkConsoleImpl` implements both the `JdkConsole` interface and the
>> `JdkConsoleProvider` interface.
>
> Do you think we could separate out this class into 2 separate
> implementations? Just like we do in the
> `jdk.internal.org.jline.JdkConsoleProviderImpl`? That could perhaps make this
> implementation similar to that other one and a bit simpler? So something like:
>
>
> package jdk.internal.io;
> public final class JdkConsoleProviderImpl implements JdkConsoleProvider {
>
> private static volatile JdkConsole INSTANCE;
>
> @Override
> public JdkConsole console(boolean isTTY, Charset charset) {
> .....
> // construct and return a (JdkConsoleImpl instance)
> }
>
> public static synchronized JdkConsole getJdkConsole(Charset cs) {
> return INSTANCE != null ? INSTANCE : new JdkConsoleImpl(cs);
> }
>
> private static final class JdkConsoleImpl implements JdkConsole {
> private final Charset charset;
> private final Object readLock;
> private final Object writeLock;
> private final Reader reader;
> private final Writer out;
> private final PrintWriter pw;
> private final Formatter formatter;
> private char[] rcb;
> private boolean restoreEcho;
> private boolean shutdownHookInstalled;
>
>
> private JdkConsoleImpl(Charset cs) {
> this.charset = cs;
> ....
> }
>
> // other methods from JdkConsole
>
> }
>
>
> }
Thanks, Jai. Yes, the reason I dropped those `final`s were they are not final.
Anyway I got rid of `JdkConsoleProviderImpl` altogether, as we know this is the
last resort impl, so no need to have `ServiceLoader` to instantiate it.
-------------
PR: https://git.openjdk.org/jdk/pull/11729
