All of the comments included below are addressed in [1]. The difference versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, updated, and re-finalized but I will hold off on that until there is a final consensus on the verbiage.
Thanks, Brian [1] http://cr.openjdk.java.net/~bpb/8139206/webrev.02/ [2] http://cr.openjdk.java.net/~bpb/8139206/webrev.01-02/ [3] https://bugs.openjdk.java.net/browse/JDK-8194956 On Jan 19, 2018, at 11:49 AM, Roger Riggs <[email protected]> wrote: > A pre-existing typo: > line 67 "{@code skip()}" should be "{@code skip(*long*)}". > > Since the public readNBytes suffices for readAllBytes, I would rename the > private readAtMostNBytes > to readNBytes and avoid the duplication of javadoc. > > Keeping the existing readAllBytes before readNBytes in the source file will > make the diff easier to follow > and the methods be in alphabetical order. On Jan 22, 2018, at 12:44 AM, Peter Levart <[email protected]> wrote: > The delegation to public method (readAllBytes -> readNBytes) should then be > documented so that subclasses know that overriding readNBytes, if needed, is > sufficient. On Jan 22, 2018, at 12:52 AM, Alan Bateman <[email protected]> wrote: > The updated version looks good. I just wonde about the "For example ..." in > the @throws OOME description. The API can't be called with a len > > Integer.MAX_VALUE so this example could be confusing - I think just drop that > sentence. > > Minor formatting in passing. At L128 and L339 it would be easier to read if > the "throws IOException" were on the previous line. Also L355 might be a bit > clear if the Math.min was indented (have to look twice to see that it's not > in the while body). On Jan 22, 2018, at 7:56 AM, Adam Petcher <[email protected]> wrote: > Possible wording, if this method can be called with large length values: > > "The total amount of memory allocated by this method is proportional to the > number of bytes read from the stream. Therefore, the method may be safely > called with very large values of {@code len}.
