pivotal-jbarrett commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562929686



##########
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##########
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-    return this.methodSuffix;
+    return methodSuffix;
   }
 
   public String getName() {
-    return this.name;
+    return name;
   }
 
-  public short getMajorVersion() {
-    return this.majorVersion;
+  public short getMajor() {
+    return major;
   }
 
-  public short getMinorVersion() {
-    return this.minorVersion;
+  public short getMinor() {
+    return minor;
   }
 
   public short getRelease() {
-    return this.release;
+    return release;
   }
 
   public short getPatch() {
-    return this.patch;
+    return patch;
   }
 
-  /**
-   * Returns a string representation for this <code>Version</code>.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-    return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-    byte[] bytes = new byte[2];
-    bytes[0] = (byte) (ordinal() >> 8);
-    bytes[1] = (byte) ordinal();
-    return bytes;
+    return productName + " " + name;
   }
 
   public static Iterable<? extends KnownVersion> getAllVersions() {
-    return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+    return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
         .collect(Collectors.toList());
   }
 
   /**
    * package-protected for use by Versioning factory
    */
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-      final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
       I am not in favor of adding parameters that are unused especially when 
the use is currently in a single place and likely not going to expand beyond 
that place, given it is package private. The caller never set anything other 
than `null` meaning that if there was no known value it wanted `null` back. I 
changed the method to do what the single use caller wanted and documented the 
behavior.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to