dschneider-pivotal commented on a change in pull request #6766:
URL: https://github.com/apache/geode/pull/6766#discussion_r690659416
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -427,79 +383,85 @@ 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 List<byte[]> getRange(AbstractSortedSetRangeOptions<?> rangeOptions)
{
+ int startIndex = getRangeIndex(rangeOptions, true);
+ if (startIndex >= getSortedSetSize() && !rangeOptions.isRev()
+ || startIndex < 0 && rangeOptions.isRev()) {
+ return Collections.emptyList();
+ }
+ int endIndex = getRangeIndex(rangeOptions, false);
- private int getIndexByLex(double score, byte[] rangeValue, boolean
isExclusive,
- boolean isMinimum) {
- AbstractOrderedSetEntry minEntry =
- new MemberDummyOrderedSetEntry(rangeValue, score, isExclusive,
isMinimum);
- return scoreSet.indexOf(minEntry);
+ return addLimitToRange(rangeOptions, startIndex, endIndex);
}
- 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 int getRangeIndex(AbstractSortedSetRangeOptions<?> rangeOptions,
boolean isStartIndex) {
+ int index;
+ Object rangeValue = isStartIndex ? rangeOptions.getStart() :
rangeOptions.getEnd();
+ boolean isExclusive =
+ isStartIndex ? rangeOptions.isStartExclusive() :
rangeOptions.isEndExclusive();
+ boolean isReverseRange = rangeOptions.isRev();
- 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);
+ if (rangeOptions instanceof SortedSetRankRangeOptions) {
Review comment:
it would be nice not to have these instanceof checks here but instead
call rangeOptions.getRangeIndex(scoreSet, isStartIndex) and let the subclasses
of AbstractSortedSetRangeOptions have different impls of this method. It looks
to me like scoreSet and isStartIndex might be the only thing you need to pass
in to the method. All the other things it consults are part of the rangeOptions
so this method would no longer need to pull them out of the options. Code the
subclasses have in common could be refactored into protected methods on
AbstractSortedSetRangeOptions
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
