refactor: address comments

Signed-off-by: Stepan Bagritsevich <[email protected]>

Author: BagritsevichStepan

src/core/string_or_view.h

@@ -6,22 +6,19 @@
 
 #include <string>
 #include <string_view>
+#include <utility>
 #include <variant>
 
 namespace dfly {
 
 class StringOrView {
  public:
   static StringOrView FromString(std::string s) {
-    StringOrView sov;
-    sov.val_ = std::move(s);
-    return sov;
+    return StringOrView{std::move(s)};
   }
 
   static StringOrView FromView(std::string_view sv) {
-    StringOrView sov;
-    sov.val_ = sv;
-    return sov;
+    return StringOrView{sv};
   }
 
   StringOrView() = default;
@@ -30,6 +27,11 @@ class StringOrView {
   StringOrView& operator=(const StringOrView& o) = default;
   StringOrView& operator=(StringOrView&& o) = default;
 
+  explicit StringOrView(std::string&& s) : val_(std::forward<std::string>(s)) {
+  }
+  explicit StringOrView(std::string_view sv) : val_(sv) {
+  }
+
   bool operator==(const StringOrView& o) const {
     return *this == o.view();
   }

src/facade/facade_types.h

@@ -10,9 +10,11 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <variant>
 
 #include "base/iterator.h"
+#include "core/string_or_view.h"
 #include "facade/op_status.h"
 
 namespace facade {
@@ -155,26 +157,26 @@ struct FacadeStats {
 
 struct ErrorReply {
   explicit ErrorReply(std::string&& msg, std::string_view kind = {})
-      : message{std::move(msg)}, kind{kind} {
+      : message{std::forward<std::string>(msg)}, kind{kind} {
   }
   explicit ErrorReply(std::string_view msg, std::string_view kind = {}) : message{msg}, kind{kind} {
   }
   explicit ErrorReply(const char* msg,
                       std::string_view kind = {})  // to resolve ambiguity of constructors above
       : message{std::string_view{msg}}, kind{kind} {
   }
-  explicit ErrorReply(OpStatus status) : message{}, kind{}, status{status} {
+  explicit ErrorReply(OpStatus status) : status{status} {
   }
 
   bool HasStatus() const {
     return status.has_value();
   }
 
   std::string_view ToSv() const {
-    return std::visit(kToSV, message);
+    return message.view();
   }
 
-  std::variant<std::string, std::string_view> message;
+  dfly::StringOrView message;
   std::string_view kind;
   std::optional<OpStatus> status{std::nullopt};
 };

src/facade/reply_builder.cc

@@ -140,10 +140,6 @@ void SinkReplyBuilder::SendError(ErrorReply error) {
   if (error.HasStatus())
     return SendError(*error.status);
 
-  SendErrorReplyMessage(error);
-}
-
-void SinkReplyBuilder::SendErrorReplyMessage(const ErrorReply& error) {
   SendError(error.ToSv(), error.kind);
 }
 

src/facade/reply_builder.h

@@ -79,8 +79,8 @@ class SinkReplyBuilder {
   }
 
   virtual void SendError(std::string_view str, std::string_view type = {}) = 0;  // MC and Redis
-  void SendError(ErrorReply error);
   virtual void SendError(OpStatus status);
+  void SendError(ErrorReply error);
 
   virtual void SendStored() = 0;  // Reply for set commands.
   virtual void SendSetSkipped() = 0;
@@ -148,8 +148,6 @@ class SinkReplyBuilder {
   static void ResetThreadLocalStats();
 
  protected:
-  virtual void SendErrorReplyMessage(const ErrorReply& error);
-
   void SendRaw(std::string_view str);  // Sends raw without any formatting.
   void SendRawVec(absl::Span<const std::string_view> msg_vec);
 

src/facade/reply_capture.cc

@@ -21,11 +21,6 @@ void CapturingReplyBuilder::SendError(std::string_view str, std::string_view typ
   Capture(Error{str, type});
 }
 
-void CapturingReplyBuilder::SendErrorReplyMessage(const ErrorReply& error) {
-  SKIP_LESS(ReplyMode::ONLY_ERR);
-  Capture(Error{error.ToSv(), error.kind});
-}
-
 void CapturingReplyBuilder::SendMGetResponse(MGetResponse resp) {
   SKIP_LESS(ReplyMode::FULL);
   Capture(std::move(resp));

src/facade/reply_capture.h

@@ -97,9 +97,6 @@ class CapturingReplyBuilder : public RedisReplyBuilder {
     std::vector<Payload> arr;
   };
 
- protected:
-  void SendErrorReplyMessage(const ErrorReply& error) override;
-
  private:
   // Send payload directly, bypassing external interface. For efficient passing between two
   // captures.