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.html
webrev: http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev/
Thanks,
Joe
