RFR [8020669] java.nio.file.Files.readAllBytes() does not read any data when Files.size() is 0

[Alan Bateman]

On 25/07/2013 16:56, Ivan Gerasimov wrote:
>
> Would you please help review an updated webrev?
> http://cr.openjdk.java.net/~igerasim/8020669/4/webrev/
Sorry for the late response to your mails on this, I was on vacation.

As you have found, in the original implementation we allowed for 
resizing (truncation or extending) while the file was open but when it 
was optimized to avoid an unnecessary resize of the array then we lost 
the ability to deal with extending case (which is the essentially the 
same as when the size is reported is less than the actual size).

Reverting to the original implementation and doing a single read to 
double check for EOF seems reasonable.

My only real comment on the latest webrev (4) is that the computation of 
the new capacity is not obvious to the reader (and since this is the 
uncommon case then it doesn't need to be super-optimized).  So I think 
it would be a lot clearer if "newCapacity" were re-introduced and then 
computed to a value that is a minimum of capacity+2, maximum of 
capacity+BUFFER_SIZE (with OOME handling when newCapacity is greater 
than MAX_BUFFER_SIZE). I think that would be clearer than re-using the 
remaining count (rem).

A minor comment is that the input stream is named "fis" but probably 
should be "in" as it is not a FileInputStream (I see David Llyod 
suggested using a FileInputStream but the path may be to a file in a 
custom file system so it can't be a FileInputStream).

I see you've changed the javadoc from "Read" to "Reads" on several 
methods. I don't know if you meant to include this in the webrev but 
personally prefer "Read".

Thanks for moving the test to BytesAndLines as that is the test for 
these methods. One suggestion is to move the new test for readAllBytes 
into testReadAndWriteBytes and the new test for readAllLines into 
testReadLines. It doesn't matter of course, it just keeps them together.

-Alan.