torwig commented on code in PR #2169:
URL: https://github.com/apache/kvrocks/pull/2169#discussion_r1524903733


##########
src/commands/cmd_stream.cc:
##########
@@ -31,6 +31,41 @@
 
 namespace redis {
 
+class CommandXAck : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    stream_name_ = args[1];
+    group_name_ = args[2];
+    StreamEntryID tmp_id;
+    for (int i = 3; i < args.size(); ++i) {
+      auto s = ParseStreamEntryID(args[i], &tmp_id);
+      if (!s.IsOK()) {
+        return {Status::RedisParseErr, s.Msg()};
+      }
+      entry_set_.emplace_back(tmp_id);
+    }
+
+    return Status::OK();
+  }
+
+  Status Execute(Server *srv, Connection *conn, std::string *output) override {
+    redis::Stream stream_db(srv->storage, conn->GetNamespace());
+    uint64_t acknowledged = 0;
+    auto s = stream_db.DeletePelEntries(stream_name_, group_name_, entry_set_, 
acknowledged);
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
+    *output = redis::Integer(acknowledged);
+
+    return Status::OK();
+  }
+
+ private:
+  std::string stream_name_;
+  std::string group_name_;
+  std::vector<StreamEntryID> entry_set_;

Review Comment:
   You can name this variable just `entries_` (or even better - `entry_ids`) 
without suffix `set/list/map/etc` - because currently the type of the variable 
is a std::vector but the name has the `set` as a suffix.



##########
src/commands/cmd_stream.cc:
##########
@@ -31,6 +31,41 @@
 
 namespace redis {
 
+class CommandXAck : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    stream_name_ = args[1];
+    group_name_ = args[2];
+    StreamEntryID tmp_id;
+    for (int i = 3; i < args.size(); ++i) {
+      auto s = ParseStreamEntryID(args[i], &tmp_id);
+      if (!s.IsOK()) {
+        return {Status::RedisParseErr, s.Msg()};
+      }
+      entry_set_.emplace_back(tmp_id);
+    }
+
+    return Status::OK();
+  }
+
+  Status Execute(Server *srv, Connection *conn, std::string *output) override {
+    redis::Stream stream_db(srv->storage, conn->GetNamespace());
+    uint64_t acknowledged = 0;
+    auto s = stream_db.DeletePelEntries(stream_name_, group_name_, entry_set_, 
acknowledged);
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
+    *output = redis::Integer(acknowledged);
+
+    return Status::OK();
+  }
+
+ private:
+  std::string stream_name_;
+  std::string group_name_;
+  std::vector<StreamEntryID> entry_set_;

Review Comment:
   You can name this variable just `entries_` (or even better - `entry_ids_`) 
without suffix `set/list/map/etc` - because currently the type of the variable 
is a std::vector but the name has the `set` as a suffix.



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