[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #1429: refactor: zset ranges

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

tisonkun commented on code in PR #1429:
URL: https://github.com/apache/incubator-kvrocks/pull/1429#discussion_r1186819875


##########
src/commands/cmd_zset.cc:
##########
@@ -356,59 +356,49 @@ class CommandZRangeGeneric : public Commander {
   }
 
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
-    if (range_type_ == kZRangeAuto || range_type_ == kZRangeRank) {
-      redis::ZSet zset_db(svr->storage, conn->GetNamespace());
-      std::vector<MemberScore> member_scores;
-      auto s = zset_db.RangeByRank(args_[1], rank_spec_, &member_scores);
-      if (!s.ok()) {
-        return {Status::RedisExecErr, s.ToString()};
-      }
+    redis::ZSet zset_db(svr->storage, conn->GetNamespace());
 
-      if (!with_scores_) {
-        output->append(redis::MultiLen(member_scores.size()));
-      } else {
-        output->append(redis::MultiLen(member_scores.size() * 2));
-      }
+    std::vector<MemberScore> member_scores;
+    std::vector<std::string> members;
 
-      for (const auto &ms : member_scores) {
-        output->append(redis::BulkString(ms.member));
-        if (with_scores_) output->append(redis::BulkString(util::Float2String(ms.score)));
-      }
+    rocksdb::Status s;
 
-      return Status::OK();
-    } else if (range_type_ == kZRangeLex) {
-      int size = 0;
-      redis::ZSet zset_db(svr->storage, conn->GetNamespace());
-      std::vector<std::string> members;
-      auto s = zset_db.RangeByLex(args_[1], lex_spec_, &members, &size);
-      if (!s.ok()) {
-        return {Status::RedisExecErr, s.ToString()};
-      }
+    switch (range_type_) {
+      case kZRangeAuto:
+      case kZRangeRank:
+        s = zset_db.RangeByRank(key_, rank_spec_, &member_scores, nullptr);
+        break;
+      case kZRangeScore:
+        s = zset_db.RangeByScore(key_, score_spec_, &member_scores, nullptr);
+        break;
+      case kZRangeLex:
+        s = zset_db.RangeByLex(key_, lex_spec_, &members, nullptr);
+        break;
+    }
+
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
 
-      *output = redis::MultiBulkString(members, false);
+    if (!members.empty()) {

Review Comment:
   Make sense. Let me think of how we can do this polymorphic thing properly...



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