[GitHub] [geode] ringles commented on a change in pull request #6766: GEODE-9389: Implement Radish ZREVRANGEBYLEX Command

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

ringles commented on a change in pull request #6766:
URL: https://github.com/apache/geode/pull/6766#discussion_r691380143



##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -427,79 +387,41 @@ long zrevrank(byte[] member) {
     return zincrby(region, key, increment, member);
   }
 
-  private int getIndexByScore(Double startRange, boolean isExclusive, boolean isMinimum) {
-    AbstractOrderedSetEntry entry =
-        new ScoreDummyOrderedSetEntry(startRange, isExclusive, isMinimum);
-    return scoreSet.indexOf(entry);
-  }
-
-  private int getIndexByLex(double score, byte[] rangeValue, boolean isExclusive,
-      boolean isMinimum) {
-    AbstractOrderedSetEntry minEntry =
-        new MemberDummyOrderedSetEntry(rangeValue, score, isExclusive, isMinimum);
-    return scoreSet.indexOf(minEntry);
-  }
-
-  private List<byte[]> getRange(int min, int max, boolean withScores, boolean isReverse) {
-    List<byte[]> result = new ArrayList<>();
-    int start;
-    int rangeSize;
-    if (isReverse) {
-      // scoreSet.size() - 1 is the maximum index of elements in the sorted set
-      start = scoreSet.size() - 1 - getBoundedStartIndex(min, scoreSet.size());
-      int end = scoreSet.size() - 1 - getBoundedEndIndex(max, scoreSet.size());
-      // Add one to rangeSize because the range is inclusive, so even if start == end, we return one
-      // element
-      rangeSize = start - end + 1;
-    } else {
-      start = getBoundedStartIndex(min, scoreSet.size());
-      int end = getBoundedEndIndex(max, scoreSet.size());
-      // Add one to rangeSize because the range is inclusive, so even if start == end, we return one
-      // element
-      rangeSize = end - start + 1;
-    }
-    if (rangeSize <= 0 || start == scoreSet.size()) {
-      return result;
+  private List<byte[]> getRange(AbstractSortedSetRangeOptions<?> rangeOptions) {
+    int startIndex = rangeOptions.getRangeIndex(scoreSet, true);
+    if (startIndex >= getSortedSetSize() && !rangeOptions.isRev()
+        || startIndex < 0 && rangeOptions.isRev()) {
+      return Collections.emptyList();
     }
+    int endIndex = rangeOptions.getRangeIndex(scoreSet, false);
 
-    Iterator<AbstractOrderedSetEntry> entryIterator =
-        scoreSet.getIndexRange(start, rangeSize, isReverse);
-    while (entryIterator.hasNext()) {
-      AbstractOrderedSetEntry entry = entryIterator.next();
-      result.add(entry.member);
-      if (withScores) {
-        result.add(entry.scoreBytes);
-      }
-    }
-    return result;
+    return addLimitToRange(rangeOptions, startIndex, endIndex);
   }
 
   private List<byte[]> addLimitToRange(AbstractSortedSetRangeOptions<?> rangeOptions,

Review comment:
       The name of this function seems to be a bit off; so long as we're refactoring like this it might make sense to rename it. "processRangeWithLimit()"? "returnRangeConstrainedByLimit()"?




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

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org