Avro
  1. Avro
  2. AVRO-142

Removing unused fields, constructors, and imports

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: java
    • Labels:
      None

      Description

      The patch to follow removes some unused fields. I've been setting up Eclipse to work on Avro (patch forthcoming), and Eclipse is perhaps unreasonably whiny about these things, but there were so few, that it seemed harmless to fix them.

      DataFileReader had a "count" field, which was never read (only assigned), so I've deleted it.

      Several classes had LOG fields that were never used; I deleted them.

      TestCompare had a handful of unused imports; just deleted them.

      1. AVRO-142.patch.text
        9 kB
        Philip Zeyliger
      2. AVRO-142.patch.v2.txt
        33 kB
        Philip Zeyliger
      3. AVRO-142.patch.v3.txt
        33 kB
        Philip Zeyliger

        Activity

        Hide
        Doug Cutting added a comment -

        For the changes to TestCompare to be maintainable, we should start running checkstyle on the test code tree. Would you like to make that change too?

        Show
        Doug Cutting added a comment - For the changes to TestCompare to be maintainable, we should start running checkstyle on the test code tree. Would you like to make that change too?
        Hide
        Doug Cutting added a comment -

        Also, the bug in DataFileReader is a missing accessor method, not an unused field.

        Show
        Doug Cutting added a comment - Also, the bug in DataFileReader is a missing accessor method, not an unused field.
        Hide
        Philip Zeyliger added a comment -

        Ok. I've added an accessor to DataFileReader, and added an assert that uses it in one of the tests. (I'll file a separate JIRA for the synchronized-ness of DataFileReader. I suspect it shouldn't be there, and it's certainly incomplete.)

        I've also started running checkstyle on the test code tree. At that point, I had to fix the following checkstyle errors, which I did as well as I could. I took the liberty of changing one use of Vector (which has synchronized access) to ArrayList, but otherwise everything here was mechanical. There was one class rename, which will be irritating in the diff.

        checkstyle-java:
        [checkstyle] Running Checkstyle 5.0 on 102 files
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/BarRecord.java:0: File does not end with a newline.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/FooRecord.java:0: File does not end with a newline.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/GenerateBlockingData.java:63:10: Name 'FILE' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/RandomData.java:30: Using the '.' form of import should be avoided - java.util..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestBulkData.java:22: Using the '.' form of import should be avoided - org.apache.avro.ipc..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestBulkData.java:23: Using the '.' form of import should be avoided - org.junit..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestNamespaceSpecific.java:37: Using the '.' form of import should be avoided - java.io..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestProtocolGeneric.java:26: Using the '.' form of import should be avoided - org.apache.avro.ipc..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestProtocolSpecific.java:39: Using the '.' form of import should be avoided - java.io..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:32:15: Name 'testEOF_boolean' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:37:15: Name 'testEOF_int' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:42:15: Name 'testEOF_long' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:47:15: Name 'testEOF_float' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:52:15: Name 'testEOF_double' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:57:15: Name 'testEOF_bytes' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:62:15: Name 'testEOF_string' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:68:15: Name 'testEOF_fixed' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:74:15: Name 'testEOF_enum' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:58:14: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:78:14: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:78:34: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:131:7: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:131:27: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:222:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:228:15: Name 'testSkip_1' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:229:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:234:15: Name 'testSkip_2' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:235:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:240:15: Name 'testSkip_3' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:241:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:247:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:253:25: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:267:25: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:281:25: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:291:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:56:15: Name 'test_identical' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:64:15: Name 'test_compatible' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:123:1: File contains tab characters (this is the first instance).
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:30:14: Name 'TestResolvingIO_resolving' must match pattern '^[A-Z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:59:15: Name 'test_resolving' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:73:5: File contains tab characters (this is the first instance).
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:34: Using the '.' form of import should be avoided - java.util..
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:37: Name '_enc' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:47: Name '_skip' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:61: Name '_js' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:73: Name '_cls' must match pattern '^[a-z][a-zA-Z0-9]*$'.
        [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:55:22: Name 'COUNT' must match pattern '^[a-z][a-zA-Z0-9]*$'.

        BTW, I've pushed my working git repository to github at http://github.com/philz/avro (branch "working"). If you're git-happy, it's sometimes an easier way to review the patch, especially with renames.

        Show
        Philip Zeyliger added a comment - Ok. I've added an accessor to DataFileReader, and added an assert that uses it in one of the tests. (I'll file a separate JIRA for the synchronized-ness of DataFileReader. I suspect it shouldn't be there, and it's certainly incomplete.) I've also started running checkstyle on the test code tree. At that point, I had to fix the following checkstyle errors, which I did as well as I could. I took the liberty of changing one use of Vector (which has synchronized access) to ArrayList, but otherwise everything here was mechanical. There was one class rename, which will be irritating in the diff. checkstyle-java: [checkstyle] Running Checkstyle 5.0 on 102 files [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/BarRecord.java:0: File does not end with a newline. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/FooRecord.java:0: File does not end with a newline. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/GenerateBlockingData.java:63:10: Name 'FILE' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/RandomData.java:30: Using the '. ' form of import should be avoided - java.util. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestBulkData.java:22: Using the '. ' form of import should be avoided - org.apache.avro.ipc. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestBulkData.java:23: Using the '. ' form of import should be avoided - org.junit. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestNamespaceSpecific.java:37: Using the '. ' form of import should be avoided - java.io. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestProtocolGeneric.java:26: Using the '. ' form of import should be avoided - org.apache.avro.ipc. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/TestProtocolSpecific.java:39: Using the '. ' form of import should be avoided - java.io. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:32:15: Name 'testEOF_boolean' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:37:15: Name 'testEOF_int' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:42:15: Name 'testEOF_long' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:47:15: Name 'testEOF_float' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:52:15: Name 'testEOF_double' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:57:15: Name 'testEOF_bytes' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:62:15: Name 'testEOF_string' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:68:15: Name 'testEOF_fixed' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBinaryDecoder.java:74:15: Name 'testEOF_enum' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:58:14: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:78:14: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:78:34: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:131:7: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:131:27: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:222:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:228:15: Name 'testSkip_1' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:229:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:234:15: Name 'testSkip_2' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:235:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:240:15: Name 'testSkip_3' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:241:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:247:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:253:25: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:267:25: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:281:25: Redundant throws: 'UnsupportedEncodingException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestBlockingIO.java:291:12: Redundant throws: 'JsonParseException' is subclass of 'IOException'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:56:15: Name 'test_identical' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:64:15: Name 'test_compatible' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO.java:123:1: File contains tab characters (this is the first instance). [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:30:14: Name 'TestResolvingIO_resolving' must match pattern '^ [A-Z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:59:15: Name 'test_resolving' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestResolvingIO_resolving.java:73:5: File contains tab characters (this is the first instance). [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:34: Using the '. ' form of import should be avoided - java.util. . [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:37: Name '_enc' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:47: Name '_skip' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:61: Name '_js' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:49:73: Name '_cls' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. [checkstyle] /Users/philip/src/avro/src/test/java/org/apache/avro/io/TestValidatingIO.java:55:22: Name 'COUNT' must match pattern '^ [a-z] [a-zA-Z0-9] *$'. BTW, I've pushed my working git repository to github at http://github.com/philz/avro (branch "working"). If you're git-happy, it's sometimes an easier way to review the patch, especially with renames.
        Hide
        Doug Cutting added a comment -

        After applying this, 'ant clean test' fails for me with:

        checkstyle-java:
        [checkstyle] Running Checkstyle 5.0 on 102 files
        [checkstyle] /home/cutting/src/avro/trunk/src/test/java/org/apache/avro/TestBulkData.java:22: Using the '.*' form of import should be avoided - org.apache.avro.ipc.*.
        [checkstyle] /home/cutting/src/avro/trunk/src/test/java/org/apache/avro/TestBulkData.java:23: Using the '.*' form of import should be avoided - org.junit.*.
        BUILD FAILED
        
        Show
        Doug Cutting added a comment - After applying this, 'ant clean test' fails for me with: checkstyle-java: [checkstyle] Running Checkstyle 5.0 on 102 files [checkstyle] /home/cutting/src/avro/trunk/src/test/java/org/apache/avro/TestBulkData.java:22: Using the '.*' form of import should be avoided - org.apache.avro.ipc.*. [checkstyle] /home/cutting/src/avro/trunk/src/test/java/org/apache/avro/TestBulkData.java:23: Using the '.*' form of import should be avoided - org.junit.*. BUILD FAILED
        Hide
        Philip Zeyliger added a comment -

        Whoops; dropped a file in the commit. ant test passes now.

        Show
        Philip Zeyliger added a comment - Whoops; dropped a file in the commit. ant test passes now.
        Hide
        Doug Cutting added a comment -

        I just committed this. Thanks, Philip.

        Show
        Doug Cutting added a comment - I just committed this. Thanks, Philip.

          People

          • Assignee:
            Philip Zeyliger
            Reporter:
            Philip Zeyliger
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development