On Tue, 7 May 2024 05:49:58 GMT, Rémi Forax <fo...@openjdk.org> wrote:
>> Please review this PR which introduces the `java.io.IO` top-level class and >> three methods to `java.io.Console` for [Implicitly Declared Classes and >> Instance Main Methods (Third Preview)]. >> >> This PR has been obtained as `git merge --squash` of a now obsolete [draft >> PR]. >> >> [Implicitly Declared Classes and Instance Main Methods (Third Preview)]: >> https://bugs.openjdk.org/browse/JDK-8323335 >> [draft PR]: https://github.com/openjdk/jdk/pull/18921 > > src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 74: > >> 72: >> 73: @Override >> 74: public String readln(String prompt) { > > this code can be simplified using an early return (and the body of the > try/catch can be reduced so it is more clear which statement can cause the > IOException) > > synchronized (writeLock) { > synchronized(readLock) { > if (!prompt.isEmpty()) { > pw.print(prompt); > pw.flush(); // automatic flushing does not cover print > } > char[] array; > try { > array = readline(false); > } catch (IOException x) { > throw new IOError(x); > } > if (array != null) { > return new String(array); > } > } > } > return null; This method started as a copy of `readLine`. In its current form, it's very evident how `readln` differs from `readLine`, and I would leave it this way for now. If the feature is standardised, we could refactor both methods together, as you suggested. If the `java.io.IO` part of the feature or the feature itself is withdrawn, then we could consider refactoring the existing `readLine`. Does it make sense? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1592281687
