RFR: 8341566: Adding factory for non-synchronized CharSequence Reader
Markus KARG
duke at openjdk.org
Sun Oct 6 15:00:34 UTC 2024
On Sun, 6 Oct 2024 14:29:09 GMT, Chen Liang <liach at openjdk.org> wrote:
>> This Pull Requests proposes an implementation for [JDK-8341566](https://bugs.openjdk.org/browse/JDK-8341566): Adding the new method `public static Reader Reader.of(CharSequence)` will return an anonymous, non-synchronized implementation of a `Reader` for each kind of `CharSequence` implementation. It is optimized for `String`, `StringBuilder`, `StringBuffer` and `CharBuffer`.
>>
>> In addition, this Pull Request proposes to replace the implementation of `StringReader` to become a simple synchronized wrapper around `Reader.of(CharSequence)` for the case of `String` sources. To ensure correctness, this PR...
>> * ...simply moved the **original code** of `StringBuilder` to become the de-facto implementation of `Reader.of()`, then stripped synchronized from it on the left hand, but kept just a synchronized wrapper on the right hand. Then added a `switch` for optimizations within the original code, at the exact location where previously just an optimization for `String` lived in.
>> * ...added tests for all methods (`Of.java`), and applied that test upon the modified `StringBuilder`.
>>
>> Wherever new JavaDocs were added, existing phrases from other code locations have been copied and adapted, to best match the same wording.
>
> src/java.base/share/classes/java/io/Reader.java line 161:
>
>> 159: * {@code read(char[])}, {@code read(char[], int, int)},
>> 160: * {@code read(CharBuffer)}, {@code ready()}, {@code skip(long)}, and
>> 161: * {@code transferTo()} methods all throw {@code IOException}.
>
> I believe these 2 paragraphs are all redundant besides the initial `The returned stream is initially open.` sentence: those are duplicating information already available from `Reader::close` specification.
>
> Closing `StringReader` is optional but its specification doesn't mention this. I think we don't need to mention if closing is optional, so that one sentence is sufficient.
This text already exists in this very file, it is a 1:1 copy from `nullReader()`. It cannot be wrong for `of()` if it was correct for `nullReader()`. To stay consistent with existing JavaDocs in this very same file, we should not have different texts for the same cases.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1789122833
More information about the core-libs-dev
mailing list