[tor-bugs] #22674 [Metrics/metrics-lib]: Consider changing instance methods to static methods

Tor Bug Tracker & Wiki blackhole at torproject.org
Tue Jun 20 18:33:25 UTC 2017


#22674: Consider changing instance methods to static methods
-------------------------------------+--------------------------
     Reporter:  karsten              |      Owner:  metrics-team
         Type:  enhancement          |     Status:  new
     Priority:  Medium               |  Milestone:
    Component:  Metrics/metrics-lib  |    Version:
     Severity:  Normal               |   Keywords:
Actual Points:                       |  Parent ID:
       Points:                       |   Reviewer:
      Sponsor:                       |
-------------------------------------+--------------------------
 There's a [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?id=fc04e540b86c7e9d375084c879c02280f8fd60ee patch] that we
 didn't merge yet that makes methods without instance references static.
 I'm including that patch here in bulk, just in case it gets lost when the
 branch gets deleted:

 {{{
 From fc04e540b86c7e9d375084c879c02280f8fd60ee Mon Sep 17 00:00:00 2001
 From: iwakeh <iwakeh at torproject.org>
 Date: Mon, 19 Jun 2017 15:08:45 +0000
 Subject: Make methods without instance references 'static'.


 diff --git
 a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java
 b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java
 index a0be85c..ec04901 100644
 ---
 a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java
 +++
 b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java
 @@ -69,7 +69,7 @@ public class DescriptorParserImpl implements
 DescriptorParser {
          NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "3"))
          && firstLines.contains(
          NL + Key.VOTE_STATUS.keyword + SP + "consensus" + NL))) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.NETWORK_STATUS_VERSION,
 RelayNetworkStatusConsensusImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type network-status-vote-3 1.")
 @@ -79,7 +79,7 @@ public class DescriptorParserImpl implements
 DescriptorParser {
          NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "3" + NL))
          && firstLines.contains(
          NL + Key.VOTE_STATUS.keyword + SP + "vote" + NL))) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.NETWORK_STATUS_VERSION, RelayNetworkStatusVoteImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type bridge-network-status 1.")
 @@ -90,42 +90,42 @@ public class DescriptorParserImpl implements
 DescriptorParser {
            descriptorFile, fileName,
 this.failUnrecognizedDescriptorLines));
        return parsedDescriptors;
      } else if (firstLines.startsWith("@type bridge-server-descriptor
 1.")) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.ROUTER, BridgeServerDescriptorImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type server-descriptor 1.")
          || firstLines.startsWith(Key.ROUTER.keyword + SP)
          || firstLines.contains(NL + Key.ROUTER.keyword + SP)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.ROUTER, RelayServerDescriptorImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type bridge-extra-info 1.")) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.EXTRA_INFO, BridgeExtraInfoDescriptorImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type extra-info 1.")
          || firstLines.startsWith(Key.EXTRA_INFO.keyword + SP)
          || firstLines.contains(NL + Key.EXTRA_INFO.keyword + SP)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.EXTRA_INFO, RelayExtraInfoDescriptorImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type microdescriptor 1.")
          || firstLines.startsWith(Key.ONION_KEY.keyword + NL)
          || firstLines.contains(NL + Key.ONION_KEY.keyword + NL)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.ONION_KEY, MicrodescriptorImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type bridge-pool-assignment 1.")
          || firstLines.startsWith(Key.BRIDGE_POOL_ASSIGNMENT.keyword + SP)
          || firstLines.contains(NL + Key.BRIDGE_POOL_ASSIGNMENT.keyword +
 SP)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.BRIDGE_POOL_ASSIGNMENT, BridgePoolAssignmentImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type dir-key-certificate-3 1.")
          || firstLines.startsWith(Key.DIR_KEY_CERTIFICATE_VERSION.keyword
 + SP)
          || firstLines.contains(
          NL + Key.DIR_KEY_CERTIFICATE_VERSION.keyword + SP)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.DIR_KEY_CERTIFICATE_VERSION,
 DirectoryKeyCertificateImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type tordnsel 1.")
 @@ -140,13 +140,13 @@ public class DescriptorParserImpl implements
 DescriptorParser {
          Key.NETWORK_STATUS_VERSION.keyword + SP + "2" + NL)
          || firstLines.contains(
          NL + Key.NETWORK_STATUS_VERSION.keyword + SP + "2" + NL)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.NETWORK_STATUS_VERSION, RelayNetworkStatusImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type directory 1.")
          || firstLines.startsWith(Key.SIGNED_DIRECTORY.keyword + NL)
          || firstLines.contains(NL + Key.SIGNED_DIRECTORY.keyword + NL)) {
 -      return this.parseDescriptors(rawDescriptorBytes, descriptorFile,
 +      return parseDescriptors(rawDescriptorBytes, descriptorFile,
            Key.SIGNED_DIRECTORY, RelayDirectoryImpl.class,
            this.failUnrecognizedDescriptorLines,
 includeUnparseableDescriptors);
      } else if (firstLines.startsWith("@type torperf 1.")) {
 @@ -158,7 +158,7 @@ public class DescriptorParserImpl implements
 DescriptorParser {
      }
    }

 -  private List<Descriptor> parseDescriptors(byte[] rawDescriptorBytes,
 +  private static List<Descriptor> parseDescriptors(byte[]
 rawDescriptorBytes,
        File descriptorFile, Key key,
        Class<? extends DescriptorImpl> descriptorClass,
        boolean failUnrecognizedDescriptorLines,
 @@ -212,7 +212,7 @@ public class DescriptorParserImpl implements
 DescriptorParser {
        int[] offsetAndLength = new int[] { startAnnotations,
            endDescriptor - startAnnotations };
        try {
 -        parsedDescriptors.add(this.parseDescriptor(rawDescriptorBytes,
 +        parsedDescriptors.add(parseDescriptor(rawDescriptorBytes,
              offsetAndLength, descriptorFile, constructor,
              failUnrecognizedDescriptorLines));
        } catch (DescriptorParseException e) {
 @@ -229,7 +229,7 @@ public class DescriptorParserImpl implements
 DescriptorParser {
      return parsedDescriptors;
    }

 -  Descriptor parseDescriptor(byte[] rawDescriptorBytes,
 +  static Descriptor parseDescriptor(byte[] rawDescriptorBytes,
        int[] offsetAndLength, File descriptorFile,
        Constructor<? extends DescriptorImpl> constructor,
        boolean failUnrecognizedDescriptorLines) throws
 DescriptorParseException {
 diff --git
 a/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java
 b/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java
 index 558a395..be86313 100644
 ---
 a/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java
 +++
 b/src/test/java/org/torproject/descriptor/impl/DescriptorParserImplTest.java
 @@ -55,8 +55,7 @@ public class DescriptorParserImplTest {
      this.thrown.expect(DescriptorParseException.class);
      this.thrown.expectMessage("'176x.158.53.63' in line 'router
 UbuntuCore169 "
          + "176x.158.53.63 44583 0 0' is not a valid IPv4 address.");
 -    DescriptorParserImpl dpi = new DescriptorParserImpl();
 -    dpi.parseDescriptor(DEFECT.getBytes(),
 +    DescriptorParserImpl.parseDescriptor(DEFECT.getBytes(),
          new int[]{0, DEFECT.getBytes().length}, null, constructor,
 false);
    }

 --
 cgit v0.10.2

 }}}

 I'm not yet sure whether that commit makes things better or worse. Let's
 discuss that here, not limited to this specific case.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22674>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list