RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

[Joe Wang]

Hi Daniel,

Thanks for reviewing the proposal!

For the spec, or javadoc in general, the first sentence shall be a short 
summary of the method, a definition of what the method is. It appears in 
the method summary table and index. So in this case, this method "Tests 
if the content of two files is identical" -- I've phrased it with words 
as Alan suggested.

I've also updated the description to hopefully make it clear that this 
method extends the existing isSameFile method, and that the process 
builds on top of that operation.

For the impl, it was an indeed quick impl. with small files in mind. But 
a memory conscience impl is always right, I've changed it to read a 
chunk at a time.

Best,
Joe


On 4/27/2018 3:58 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> On the specification side, I think I would reword the API
> documentation to first explain how the method checks the
> content of the two files.
>
> The fact that it doesn't check the actual content if
> the two files are 'the same' is kind of an optimization.
>
> So I would suggest to invert the order of the two paragraph
> in the documentation, and combine them into one - something like:
>
> 1536      * <p>
>           * This method first calls {@link 
> #isSameFile(java.nio.file.Path, java.nio.file.Path) isSameFile(path, 
> path2)} to determine whether the two files are the same.
> 1537      * If {@code isSameFile(path, path2)} returns false, this 
> method will proceed
> 1538      * to read the files and compare them byte by byte to 
> determine if they contain
> 1539      * the same contents.
>           * Otherwise, this method will return true without further
>           * processing.
>
>
> On the implementation side I don't think it's reasonable to call
> readAllBytes() and hold the content of the two files in memory
> for comparing their content, especially if it's to discover that
> the first byte differs.
>
> Some lock-step reading of the two files would seem more appropriate.
>
> best regards,
>
> -- daniel
>
>
>
>
> On 27/04/2018 05:51, Joe Wang wrote:
>> Hi,
>>
>> Considering extending isSameFile to add isSameContent to Files. 
>> Please review.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8202285
>>
>> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8202285/webrev/
>>
>> specdiff: 
>> http://cr.openjdk.java.net/~joehw/jdk11/8202285/specdiff/java/nio/file/Files.html 
>>
>>
>> Thanks,
>> Joe
>>
>