Hi Brian,

Looking good.

I would add a message to the thrown IOException @598:  "Unable to skip exactly".

Adding a @see to skip(n) would be a good addition to recommend using skipNBytes.

Thanks, Roger

On 11/14/2018 08:21 PM, Brian Burkhalter wrote:
Hi Brent / Daniel,

On Nov 9, 2018, at 4:41 PM, Brent Christian <brent.christ...@oracle.com> wrote:

On 11/9/18 2:04 PM, Brian Burkhalter wrote:
An updated patch is at
http://cr.openjdk.java.net/~bpb/6516099/webrev.06/ 
<http://cr.openjdk.java.net/~bpb/6516099/webrev.06/> 
<http://cr.openjdk.java.net/~bpb/6516099/webrev.06/ 
<http://cr.openjdk.java.net/~bpb/6516099/webrev.06/>>
including a revision of the implementation to align with the words. The tests 
are not updated yet.
I think this looks quite good.

My only comment is that

568      * If {@code n} is zero or negative, then no bytes are skipped.

seems a bit redundant, given the opening paragraph.  I could take it or leave 
it.

If the content of the @implSpec is supposed to be self-contained, then I think 
it should remain. As that does not appear to be the case, I agree that it could 
be deleted.

On Nov 12, 2018, at 2:48 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:

But I wonder if skip(n) returning a negative value
(ns < 0) when n has been asserted to be > 0 should be
considered as an error, and if an IOException should
be thrown?
I tend to agree that covering this sort of aberration is probably a good idea.

I have updated the patch per the foregoing comments and have also improved the 
Skip test:

http://cr.openjdk.java.net/~bpb/6516099/webrev.07/

Thanks,

Brian

Reply via email to