On Tue, 6 May 2025 15:43:07 GMT, Brian Burkhalter <b...@openjdk.org> wrote:
>> Implement the requested methods and add a test thereof.
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8354724: Revert BufferedReader; simplify Reader changes removing
> overridable self-call; update test
This is right pair of methods on Reader to cover the use cases.
The names are clear and concise.
Some editorial suggestions are included to the text, use as you see fit.
I think its clear and complete enough to get the CSR drafted and into the
proposed queue for review for JDK 25.
src/java.base/share/classes/java/io/Reader.java line 438:
> 436: *
> 437: * @throws IOException If an I/O error occurs
> 438: *
Editorial suggestion:
Literally/liberally use the description of lines and terminators from
String.lines().
Move the implementation limit to the @apiNote and reduce to the maximum size of
UTF16 strings.
Added the @throw for OOME, to make it clear there is a limit that also applies
to readAllLines().
Suggestion:
/**
* Reads all remaining characters as lines of text.
* <p>
* A <i>line</i> is either a sequence of zero or more characters
* followed by a line terminator, or it is a sequence of one or
* more characters followed by the end of the stream.
* A line does not include the line terminator.
* <p>
* A <i>line terminator</i> is one of the following:
* a line feed character {@code "\n"} (U+000A),
* a carriage return character {@code "\r"} (U+000D),
* or a carriage return followed immediately by a line feed
* {@code "\r\n"} (U+000D U+000A).
*
* <p> The method does not close this reader nor its underlying stream.
* If an I/O error occurs, the states of the reader and its underlying
* stream are unspecified.
*
* @apiNote
* This method is intended for simple cases where it is convenient
* to read all remaining characters in a single operation. It is not
* intended for reading a large number of characters, for example, greater
than 1G.
*
* @return the remaining characters as lines of text stored in an
* unmodifiable {@code List} of strings in the order they are
read
*
* @throws IOException If an I/O error occurs
* @throws OutOfMemoryError If the number of remaining characters
exceeds the
* implementation limit for String.
*
src/java.base/share/classes/java/io/Reader.java line 470:
> 468: * @throws OutOfMemoryError If the remaining content is
> extremely
> 469: * large, for example larger than
> {@code 2GB}
> 470: *
Editorial suggestions:
Change 'content' to 'characters' to match rest of descriptions.
Move mention of implementation limit, 1G to the @apiNote
Changed the @throw OOME to just say it exceeds the implementation limit of
string.
Suggestion:
/**
* Reads all remaining characters into a string.
*
* <p> This method reads all remaining characters including all line
separators
* to the end of the stream. The resulting string will contain line
* separators as they appear in the stream.
*
* <p> The method does not close this reader nor its underlying stream.
* If an I/O error occurs, the states of the reader and its underlying
* stream are unspecified.
*
* @apiNote
* This method is intended for simple cases where it is appropriate and
* convenient to read all remaining characters into a String. It is not
* intended for reading a large number of characters, for example, greater
than 1G.
*
* @return a String containing all remaining characters
*
* @throws IOException If an I/O error occurs
* @throws OutOfMemoryError If the number of remaining characters
exceeds the
* implementation limit for String.
*
-------------
PR Review: https://git.openjdk.org/jdk/pull/24728#pullrequestreview-2829623531
PR Review Comment: https://git.openjdk.org/jdk/pull/24728#discussion_r2082474638
PR Review Comment: https://git.openjdk.org/jdk/pull/24728#discussion_r2082474398
