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

Ivan Gerasimov ivan.gerasimov at oracle.com
Wed Nov 7 19:39:04 UTC 2018


Thank you Joe!

I like the last variant.

With kind regards,
Ivan

On 11/7/18 9:59 AM, Joe Wang wrote:
> 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-bounces at openjdk.java.net> On 
>>>> Behalf Of Roger Riggs
>>>> Sent: Tuesday, November 6, 2018 8:56 AM
>>>> To: core-libs-dev at 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
>>>
>>
>

-- 
With kind regards,
Ivan Gerasimov



More information about the core-libs-dev mailing list