On Sat, 5 Oct 2024 16:32:39 GMT, Markus KARG <d...@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 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. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21371#issuecomment-2395436600
