RFR: 8341566: Adding factory for non-synchronized CharSequence Reader

[Alan Bateman]

On Sun, 6 Oct 2024 13:14:32 GMT, Alan Bateman <alanb 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.
>
> Reader.of(CharSequence) looks much better than introducing CharSequenceReader. It won't have an equivalent on Writer but I think that is okay. Also it means that the user will need to deal with close throwing IOException but anything using Reader has to do this already.
> 
> I think it would be better to drop "API compatible with StringReader" from the method description. An apiNote in StringReader can direct readers to the static factory method.
> 
> Also I think drop the  "lock" field from the API docs as it's a protected field and only interesting to subclasses.  The Reader class does not specify if Reader is thread-safe so the method description doesn't need to say too much. For clarify then it could just say something like "the resulting Reader is not safe for use by multiple concurrent threads. If the Reader is to be used by more than one thread it should be controlled by appropriate synchronization".
> 
> The parameter name is currently "c", maybe you mean "cs"? The method will need to specify NPE for the of(null) case.

> @AlanBateman Can you please review [the CSR request](https://bugs.openjdk.org/browse/JDK-8341596) so I can finish it? Thanks!

Latest API docs looks good, will you update the CSR?

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

PR Comment: https://git.openjdk.org/jdk/pull/21371#issuecomment-2401489513