RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file
Alan.Bateman at oracle.com
Thu Jun 14 11:30:29 UTC 2018
On 12/06/2018 17:52, Joe Wang wrote:
> Hi all,
> It's been a while since the 1st round of reviews. Thank you all for
> the valuable comments and suggestions! Note that Roger's last
> response went to nio-dev, but not core-libs-dev, I've therefore
> attached it below.
> CSR : the CSR is now approved. Note the write method has been
> changed to writeString.
> Impl : for performance reason and the different requirement from
> byte-string conversion for malformed/unmappable character handing, the
> implementation uses a specific method separate from the existing ones.
> Both Sherman and Alan preferred specific method than adding parameters
> to the APIs that might be error prone. Thanks Sherman for the code!
> JMH benchmark tests showed the new read method performs better than an
> operation that reads the bytes and convert to string. The gains start
> to be significant even at quite reasonable sizes (< 10K).
>  JBS: https://bugs.openjdk.java.net/browse/JDK-8201276
>  CSR: https://bugs.openjdk.java.net/browse/JDK-8202055
>  webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8201276/webrev/
> Please review.
The javadoc looks good.
One part of the implementation that could be cleaner is the exception
thrown for the malformed or unmappable cases. There are sub-classes of
CharacterCodingException defined for this that would be clearer than an
IOException with an IllegalArgumentException as cause.
A minor nit in Files is that you can import
jdk.internal.misc.JavaLangAccess rather than repeating the fully
qualified class name. You can also move the definition of JLA up to the
top. There's also a few inconsistencies with the existing code that
would be goof to fix too (indenting and line length issues mostly).
The test looks reasonable. In getData() then then "shouldn't happen"
case should throw an exception as a NPE here might be tricky to diagnose
there. Another nit is the sb field - can that be removed.
More information about the nio-dev