[GitHub] [geode] jdeppe-pivotal commented on a change in pull request #6530: GEODE-9255: Remove remaining uses of ByteArrayWrapper

[GitBox <gi...@apache.org>]

jdeppe-pivotal commented on a change in pull request #6530:
URL: https://github.com/apache/geode/pull/6530#discussion_r641004432



##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisKey.java
##########
@@ -63,6 +66,78 @@ public int getBucketId() {
     return getCrc16() & (REDIS_SLOTS - 1) / REDIS_SLOTS_PER_BUCKET;
   }
 
+  /**
+   * This is a byte to byte comparator, it is not lexicographical but purely compares byte by byte
+   * values
+   */
+  @Override
+  public int compareTo(RedisKey other) {
+    return arrayCmp(value, other.value);
+  }
+
+  /**
+   * Private helper method to compare two byte arrays, A.compareTo(B). The comparison is basically
+   * numerical, for each byte index, the byte representing the greater value will be the greater
+   *
+   * @param A byte[]
+   * @param B byte[]
+   * @return 1 if A > B, -1 if B > A, 0 if A == B
+   */
+  private int arrayCmp(byte[] A, byte[] B) {
+    if (A == B) {
+      return 0;
+    }
+    if (A == null) {
+      return -1;
+    } else if (B == null) {
+      return 1;
+    }
+
+    int len = Math.min(A.length, B.length);
+
+    for (int i = 0; i < len; i++) {
+      byte a = A[i];
+      byte b = B[i];
+      int diff = a - b;
+      if (diff > 0) {
+        return 1;
+      } else if (diff < 0) {
+        return -1;
+      }
+    }
+
+    if (A.length > B.length) {
+      return 1;
+    } else if (B.length > A.length) {
+      return -1;
+    }
+
+    return 0;
+  }
+
+  /**
+   * Hash code for byte[] wrapped by this object
+   */
+  @Override
+  public int hashCode() {
+    return Arrays.hashCode(value);
+  }
+
+  /**
+   * This equals is not symmetric and therefore not transitive, because a String with the same
+   * underlying bytes is considered equal. Clearly calling {@link String#equals(Object)} would not
+   * yield the same result
+   */
+  @Override
+  public boolean equals(Object other) {
+    if (other instanceof RedisKey) {
+      return Arrays.equals(value, ((RedisKey) other).value);
+    } else if (other instanceof String) {
+      return Arrays.equals(value, Coder.stringToBytes((String) other));

Review comment:
       I'm not 100%, but I think this check can be removed (at least all integration tests pass without it). I can't think where we might be comparing a `RedisKey` to a string.




-- 
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:
users@infra.apache.org