On Wed, 14 May 2025 17:01:30 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8354724: "stream" -> "reader"
>
> 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!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24728#discussion_r2089409620
