On Sun, 6 Oct 2024 14:20:12 GMT, Chen Liang <li...@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 152:
> 
>> 150:      * reader was being used by a single thread (as is generally the 
>> case).
>> 151:      * Where possible, it is recommended that this class be used in 
>> preference
>> 152:      * to {@code StringReader} as it will be faster under most 
>> implementations.
> 
> I believe this paragraph is redundant, now that `StringReader` is a 
> somewhat-deprecated API that we aim to migrate away from. We can note these 
> differences in the API notes of `StringReader`.

As Alan already proposed, I am just removing it in this very minute.

> src/java.base/share/classes/java/io/Reader.java line 174:
> 
>> 172:      * @since 24
>> 173:      */
>> 174:     public static Reader of(CharSequence c) {
> 
> Should we give this factory a more specific name so we don't clash in the 
> future? For example, if we add another factory `of(A a)` for interface `A`, 
> then it would be confusing to have an instance of `interface C extends 
> CharSequence, A` to be passed to `of`.

I do not see what is "confusing" in that case, as the caller still gets what he 
intends: a reader for the passed source. I also wonder how likely that case 
actually is. Anyways, I may be biased as I proposed `of`.

@AlanBateman WDYT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1789120245
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1789119975

Reply via email to