Refactor error handling in result class

GregoryKogan · Jan 7, 2024 · 6955baf · 6955baf
1 parent 8ef9ab9
commit 6955baf
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 6 deletions.
diff --git a/result.h b/result.h
@@ -33,12 +33,15 @@ template <typename E> class err {
   explicit err(E error) : error_(std::move(error)) {}
 
   template <typename T> operator result<T, E>() const; // NOLINT(google-explicit-constructor)
+  operator result<void, E>() const;                    // NOLINT(google-explicit-constructor)
 };
 
 template <typename E> template <typename T> inline err<E>::operator result<T, E>() const {
   return result<T, E>(error_, false);
 }
 
+template <typename E> inline err<E>::operator result<void, E>() const { return result<void, E>(error_); }
+
 } // namespace res
 
 #include <variant>
@@ -109,7 +112,7 @@ template <typename E> class result<void, E> {
   E error_;
 
   explicit result() : successful_(true) {}
-  explicit result(const E &error) : error_(error), successful_(false) {}
+  explicit result(E error) : error_(std::move(error)), successful_(false) {}
 
   template <typename U> friend class ok;
   template <typename U> friend class err;
@@ -123,12 +126,17 @@ template <typename E> class result<void, E> {
 };
 
 template <typename T, typename E> inline auto result<T, E>::value() const -> const T & {
-  if (!is_ok()) { throw std::runtime_error("value() called on result with error"); }
+  if (!is_ok()) { throw std::logic_error("value() called on result with error"); }
   return value_;
 }
 
 template <typename T, typename E> inline auto result<T, E>::error() const -> const E & {
-  if (is_ok()) { throw std::runtime_error("error() called on result with value"); }
+  if (is_ok()) { throw std::logic_error("error() called on result with value"); }
+  return error_;
+}
+
+template <typename E> inline auto result<void, E>::error() const -> const E & {
+  if (is_ok()) { throw std::logic_error("error() called on result with value"); }
   return error_;
 }
 

diff --git a/src/err/err.hpp b/src/err/err.hpp
@@ -16,10 +16,13 @@ template <typename E> class err {
   explicit err(E error) : error_(std::move(error)) {}
 
   template <typename T> operator result<T, E>() const; // NOLINT(google-explicit-constructor)
+  operator result<void, E>() const;                    // NOLINT(google-explicit-constructor)
 };
 
 template <typename E> template <typename T> inline err<E>::operator result<T, E>() const {
   return result<T, E>(error_, false);
 }
 
+template <typename E> inline err<E>::operator result<void, E>() const { return result<void, E>(error_); }
+
 } // namespace res
diff --git a/src/result/result.hpp b/src/result/result.hpp
@@ -45,7 +45,7 @@ template <typename E> class result<void, E> {
   E error_;
 
   explicit result() : successful_(true) {}
-  explicit result(const E &error) : error_(error), successful_(false) {}
+  explicit result(E error) : error_(std::move(error)), successful_(false) {}
 
   template <typename U> friend class ok;
   template <typename U> friend class err;
@@ -59,12 +59,17 @@ template <typename E> class result<void, E> {
 };
 
 template <typename T, typename E> inline auto result<T, E>::value() const -> const T & {
-  if (!is_ok()) { throw std::runtime_error("value() called on result with error"); }
+  if (!is_ok()) { throw std::logic_error("value() called on result with error"); }
   return value_;
 }
 
 template <typename T, typename E> inline auto result<T, E>::error() const -> const E & {
-  if (is_ok()) { throw std::runtime_error("error() called on result with value"); }
+  if (is_ok()) { throw std::logic_error("error() called on result with value"); }
+  return error_;
+}
+
+template <typename E> inline auto result<void, E>::error() const -> const E & {
+  if (is_ok()) { throw std::logic_error("error() called on result with value"); }
   return error_;
 }
 

diff --git a/tests/boolean_operations.cpp b/tests/boolean_operations.cpp
@@ -10,6 +10,7 @@ TEST(BooleanOperations, Ok) {
   EXPECT_TRUE(result.is_ok());
   EXPECT_FALSE(!result.is_ok());
   EXPECT_EQ(result.value(), 42);
+  EXPECT_THROW(auto val = result.error(), std::logic_error); // NOLINT
 }
 
 TEST(BooleanOperations, Err) {
@@ -20,6 +21,7 @@ TEST(BooleanOperations, Err) {
   EXPECT_FALSE(result.is_ok());
   EXPECT_TRUE(!result.is_ok());
   EXPECT_EQ(result.error(), "error");
+  EXPECT_THROW(auto val = result.value(), std::logic_error); // NOLINT
 }
 
 TEST(BooleanOperations, VoidOk) {
@@ -28,4 +30,16 @@ TEST(BooleanOperations, VoidOk) {
   EXPECT_FALSE(!result);
   EXPECT_TRUE(result.is_ok());
   EXPECT_FALSE(!result.is_ok());
+  EXPECT_THROW(auto val = result.error(), std::logic_error); // NOLINT
+}
+
+TEST(BooleanOperations, VoidErr) {
+  const std::string error = "error";
+  res::result<void, std::string> result = res::err(error);
+  EXPECT_FALSE(result);
+  EXPECT_TRUE(!result);
+  EXPECT_FALSE(result.is_ok());
+  EXPECT_TRUE(!result.is_ok());
+  EXPECT_EQ(result.error(), "error");
+  // EXPECT_THROW(auto val = result.value(), std::logic_error); - Should not compile
 }