[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1444: Add support of the new command SINTERCARD(Redis 7)

["PragmaTwice (via GitHub)" <gi...@apache.org>]

PragmaTwice commented on code in PR #1444:
URL: https://github.com/apache/incubator-kvrocks/pull/1444#discussion_r1225749446


##########
src/types/redis_set.cc:
##########
@@ -332,21 +332,21 @@ rocksdb::Status Set::Union(const std::vector<Slice> &keys, std::vector<std::stri
  * key3 = {a,c,e}
  * INTER key1 key2 key3 = {c}
  */
-rocksdb::Status Set::Inter(const std::vector<Slice> &keys, std::vector<std::string> *members, uint64_t *cnt) {
+rocksdb::Status Set::Inter(const std::vector<Slice> &keys, std::vector<std::string> *members, uint64_t limit,
+                           uint64_t *cnt) {
   members->clear();
 
   std::map<std::string, size_t> member_counters;
   std::vector<std::string> target_members;
   auto s = Members(keys[0], &target_members);
   if (!s.ok() || target_members.empty()) return s;
 
-  uint64_t limit = cnt ? *cnt : 0;
   bool has_limit = limit != 0;
-  bool limited = false;
+  bool limit_reached = false;
   size_t keys_size = keys.size();
 
   if (keys_size == 1 && has_limit) {
-    if (limit > target_members.size()) *cnt = target_members.size();
+    *cnt = limit > target_members.size() ? target_members.size() : limit;

Review Comment:
   Here maybe a nullptr check is needed for `cnt`. But IMHO, we can assume that `cnt` can never be nullptr, remove all these check and the default argument of `cnt` in the function declaration.



-- 
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: issues-unsubscribe@kvrocks.apache.org

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