Hi Daniel, You're right, line 115 is not needed. I'll add a couple of test cases to cover the route. Best, Joe On 9/20/18, 2:36 AM, Daniel Fuchs wrote:
Hi Joe,
The spec reads very well to me.
On the implementation side:
114 } else if (nRead1 != nRead2) { 115 int len = Math.min(nRead1, nRead2); 116 // there's always a mismatch when nRead1 != nRead2 117 return totalRead + 118 Arrays.mismatch(buffer1, 0, len, buffer2, 0, len);
I believe line 115 is not needed, and line 118 should be:
Arrays.mismatch(buffer1, 0, nRead1, buffer2, 0, nRead2);
otherwise - if I'm not mistaken - there's the chance that
Arrays.mismatch(buffer1, 0, len, buffer2, 0, len);
will return -1. Or am I missing something?
If I'm right then your test is probably missing a case to explicitely test for that. Alternatively you could add a whitebox test to test FilesMismatch.mismatch(InputStream, InputStream) directly.
best regards,
-- daniel
On 19/09/2018 19:48, Joe Wang wrote:
Hi,
After much discussion and 10 iterations of reviews, this proposal has evolved from what was the original isSameContent method to a mismatch method. API-wise, a compare method was also considered as it looked like just a short step forward from mismatch, however, it was eventually dropped since there is no convincing use case comparing files lexicographically by contents. Impl-wise, extensive performance benchmarking has been done to compare a buffered reading vs memory mapping, the result was that a simple buffered reading performed better among small files, and those with the mismatched byte closer to the beginning of files. Since the proposed method's targeted files are small ones, the impl currently does a buffered reading only.
Please review.
JBS: https://bugs.openjdk.java.net/browse/JDK-8202285
specdiff: http://cr.openjdk.java.net/~joehw/jdk12/8202285/specdiff/java/nio/file/Files...
webrev: http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev/
Thanks, Joe