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

Stuart Marks stuart.marks at oracle.com
Wed Oct 17 22:03:21 UTC 2018


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.

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. :-)

(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.

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.

s'marks


More information about the core-libs-dev mailing list