RFR(JDK 12/NIO) 8202285: (fs) Add a method to Files for comparing file contents
Joe Wang
huizhe.wang at oracle.com
Fri Oct 19 18:26:01 UTC 2018
On 10/17/18, 3:03 PM, Stuart Marks wrote:
> Hi Joe,
>
>> webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v05/
>
> I concur with Roger's comments on the test file generation. It would
> be good to find some way to reduce the redundancy. He had some
> suggestions that might work. You might also want to play around with
> variations to see how they look.
>
> 375 byte[] bytes = "ABCDEFGHIJKLMNOPQRSTUVWXYZ
> abcdefghijklmnopqrstuvwxyz 0123456789 \n".getBytes();
>
> A small thing, but this uses the default charset, which is
> platform-specific. I think it would be good to use
> StandardCharsets.US_ASCII here.
ASCII it is.
>
> Once you have this in bytes, it looks like all the other parts of the
> test deal with bytes (not characters) so I don't think there are any
> other concerns about charsets and such.
>
> 345 /**
> 346 * Creates a file with alphanumeric content with one
> character altered
> 347 * at the specified position.
> 348 *
> 349 * @param dir the directory in which the file is to be created
> 350 * @param purpose the purpose of the file
> 351 * @param size the size of the file
> 352 * @param pos the position where the alternative char is to
> be added. If it
> 353 * is smaller than zero, no alternation shall be
> made.
> 354 * @param c the character
> 355 * @return path of the created file
> 356 * @throws IOException
> 357 */
> 358 private static Path createANFile(Path dir, String purpose,
> int size, int pos,
> 359 byte c) throws IOException {
> 360 Path path = Files.createFile(Paths.get(dir.toString(),
> purpose + ".txt"));
> 361 if (size > 0) {
> 362 writeAlphanumericFile(path, size, pos, c);
> 363 }
> 364 return path;
> 365 }
>
> The problem with documenting things well is that the doc gets out of
> date. :-)
Very true, now I get to keep the doc as we switch back to char :-)
>
> (Well, actually somewhat serious.)
>
> This method creates test data files in terms of bytes (although the
> bytes happen to be derived from ASCII characters). The altered data is
> a byte at a particular byte offset, not a character. Several places in
> the doc need to be updated to reflect this, since they still talk
> about characters.
>
> Even though the altered data is a byte, for the convenience of the
> caller, the parameter should be a char or an int. All the places where
> this is called use a char literal casted to a byte:
>
> 125 test1024m1014a = createANFile(testDir, "test1024m1014a",
> size, size - 10, (byte)'@');
>
> If the parameter were a char or a int, the caller could just do:
>
> 125 test1024m1014a = createANFile(testDir, "test1024m1014a",
> size, size - 10, '@');
>
> The byte used to alter the test data could be the low order 7 bits
> (since we're using ASCII) of the parameter value. Either document it
> this way and simply mask off the high order bits of the parameter
> value, or assert that those high order bits are all zeroes.
Sounds good, the latter looks better with a char. It's a plus as I can
keep the statement that says it alters one character. While only the low
order byte matters for this test data (ASCII only), it's masked off to
be clear about the intention.
>
> In writeAlphanumericFile(),
>
> 370 if (pos > 0) a[pos] = c;
>
> This should check should be for pos >= 0, since altering a byte at
> position 0 is a valid test case. You might want to add this as another
> test case.
Changed to >=0, and added test cases for an altered byte at position 0
(as well as at the end).
Current version:
http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v06/
Previous:
http://cr.openjdk.java.net/~joehw/jdk12/8202285/webrev_v05/
Thanks,
Joe
>
> s'marks
More information about the core-libs-dev
mailing list