Hi Roger,

> On Oct 16, 2018, at 12:53 PM, Roger Riggs <[email protected]> wrote:
> 
> InputStream.java:
> 
> 584:  Is there really a case where line 585 would throw the exception?
> 
>   skip(n, true) != n
> 
> With the 2nd argument = true, it will either skip the bytes or throw.
> I think you can just call:  "skip(n, true);

I think you are correct.

> Given subclasses of InputStream that might override skip(n) with a more 
> efficient mechanism
> would it make sense to implement skipNBytes in terms of skip(n)?
> As is, it always uses read() to skip the bytes, which might not be as 
> efficient as possible
> on some streams (for example a very large file) where a seek could be used.

That’s a good point. I had some trepidation about the vague wording of skip(n) 
in terms of how many bytes it actually reads.

> test/jdk/java/io/InputStream/Skip.java:
> 
> There is a lot of whitespace in this test, spaces before "," and extra blank 
> lines.

Yes it is rather ugly. I chose to leave it as is for initial clarity intending 
to clean it up later.

> 97: "possible result" - does that occur in the tests?  The "possible" is a 
> bit ambiguous especially given the very narrow test case. It would be useful 
> to include the message from the exception.

Copy-paste garbage. I’ll update it to include the message.

Thanks,

Brian

Reply via email to