Thanks Ivan!
I agree that the upfront edge case checks aren't really necessary, after
all, the great majority of the use cases won't hit the edge case (with
the size being a factor of the buffer). We're therefore better off
without the checks.
Here's an updated webrev:
http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v08/
Best,
Joe
On 11/6/18, 4:21 PM, Ivan Gerasimov wrote:
Hi Joe!
I apologize, if it was already discussed before.
Why do you need to have 3 edge cases of nRead1 and nRead2 in lines
1596-1605: Was it to save a call to Arrays.mismatch()?
If I'm not mistaken, the code would work equally well, if lines
1596-1615 were replaced with just subrange 1606-1614.
With kind regards,
Ivan
On 11/6/18 12:53 PM, Joe Wang wrote:
Thanks Andrew for pointing that out. It wasn't intentional, a result
of copy n paste after removing the support class. It's fixed now:
http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v07/
-Joe
On 11/6/18, 10:27 AM, Andrew Luo wrote:
I'm not a reviewer, but just wanted to point out that the fully
qualified Files.BUFFER_SIZE can be simplified to just BUFFER_SIZE.
Searching through the code in Files.java I don't see many other
instances of using Files.* (although I do see a few).
Thanks
Andrew
-----Original Message-----
From: core-libs-dev<core-libs-dev-boun...@openjdk.java.net> On
Behalf Of Roger Riggs
Sent: Tuesday, November 6, 2018 8:56 AM
To: core-libs-dev@openjdk.java.net
Subject: Re: RFR(JDK 12/NIO) 8202285: (fs) Add a method to Files for
comparing file contents
+1
Though with 65 tests, I suspect that there are more cases than
strictly needed to cover all the code flows.
For example, three cases for testing when it is the same file
doesn't seem necessary.
Regards, Roger
On 11/05/2018 11:27 PM, Stuart Marks wrote:
On 10/19/18 11:26 AM, Joe Wang wrote:
Current version:
http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v06/
Hi Joe,
Thanks for updating the tests per my comments. Everything looks
good now!
s'marks
