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