DonalEvans commented on a change in pull request #7219:
URL: https://github.com/apache/geode/pull/7219#discussion_r778999886



##########
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -323,18 +339,48 @@ public long zinterstore(RegionProvider regionProvider, 
RedisKey key, List<ZKeyWe
           }
         }
       }
+
       if (addToSet) {
-        memberAdd(member, newScore);
+        OrderedSetEntry entry = interMembers.get(member);
+        if (entry == null) {
+          entry = new OrderedSetEntry(member, newScore);
+          interMembers.put(member, entry);
+          interScores.add(entry);
+        } else if (entry.getScore() != newScore) {
+          interScores.remove(entry);
+          entry.updateScore(newScore);
+          interScores.add(entry);
+        }
       }
     }
 
-    if (removeFromRegion()) {
-      regionProvider.getDataRegion().remove(key);
-    } else {
-      regionProvider.getLocalDataRegion().put(key, this);
+    return sortedSetOpStoreResult(regionProvider, key, interMembers, 
interScores);
+  }
+
+  @VisibleForTesting
+  static long sortedSetOpStoreResult(RegionProvider regionProvider, RedisKey 
destinationKey,
+      MemberMap interMembers, ScoreSet interScores) {
+    RedisSortedSet destinationSet =
+        regionProvider.getTypedRedisDataElseRemove(REDIS_SORTED_SET, 
destinationKey, false);
+
+    if (interMembers == null || interScores.isEmpty()) {

Review comment:
       Use of null as an argument is generally considered to be a "code smell" 
as outlined 
[here](https://medium.com/thinkster-io/code-smell-null-check-a0c4851fafbf) and 
[here](https://dzone.com/articles/code-smells-null). For the `MemberMap` on 
line 299, since we know it's empty and won't have anything added to it, just 
using a size of 0 would be fine.




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


Reply via email to