RFR: 8354724: Methods in java.io.Reader to read all characters and all lines [v19]

[Brian Burkhalter]

On Wed, 14 May 2025 17:25:00 GMT, Markus KARG <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/io/Reader.java line 403:
>> 
>>> 401:         char[] str = new char[TRANSFER_BUFFER_SIZE];
>>> 402:         int n;
>>> 403:         while ((n = read(str)) != -1) {
>> 
>> I'd suggest changing this call to `this.read(str, 0, str.length)` so that all concrete implementations in this class call only the abstract three-arg read() method. Using `this` is optional but it emphasizes that this is a self-call. (It's done in only one other place in this file already.)
>> 
>> Also, the variable name `str` is confusing since this isn't a String. I'd suggest `cbuf` instead as that name is used for char arrays elsewhere.
>
> I think `read(str)` is to be preferred over `read(str, 0, str.length()` as it allows implementations to provide optimized implementations. What the caller of the new methods wants to achieve is getting everything in the best possible way, which is, in the most optimized way. If we go around high-level methods, we should have a good reason for!

All other self-calls to a `read` method are to the three-arg abstract method:

$ grep read( src/java.base/share/classes/java/io/Reader.java | grep -v public | grep -v *
            nread = this.read(cbuf, off, rem);
            nread = read(cbuf, 0, len);
        if (read(cb, 0, 1) == -1)
        return read(cbuf, 0, cbuf.length);
        while ((nread = read(cbuf, 0, cbuf.length)) != -1) {
                int nc = read(skipBuffer, 0, (int)Math.min(r, nn));
        while ((nRead = read(buffer, 0, TRANSFER_BUFFER_SIZE)) >= 0) {

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24728#discussion_r2089432945