Refactor Try<T> leveraging Option<T> to remove dynamic allocation.
Aggregate an Option<T> to leverage the RAII pattern around unrestricted union. Added some comments to Try<T>. Review: https://reviews.apache.org/r/34277 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/18479776 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/18479776 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/18479776 Branch: refs/heads/master Commit: 18479776e65c31f14c06d77f2b830768b566b034 Parents: d9484de Author: Joris Van Remoortere <[email protected]> Authored: Tue May 26 09:50:31 2015 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Tue May 26 09:50:31 2015 -0700 ---------------------------------------------------------------------- .../3rdparty/stout/include/stout/try.hpp | 91 ++++++++------------ .../3rdparty/stout/tests/some_tests.cpp | 3 + 2 files changed, 40 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/18479776/3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp index 8150b70..bf1540b 100644 --- a/3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp @@ -21,92 +21,75 @@ #include <stout/abort.hpp> #include <stout/error.hpp> - +#include <stout/option.hpp> +#include <stout/some.hpp> + +// This class can represent only one of these states at a time: +// 1) A value of T. +// 2) An error state, with a corresponding error string. +// Calling 'isSome' will return true if it stores a value, in which +// case calling 'get' will return a constant reference to the T +// stored. Calling 'isError' will return true if it stores an error, +// in which case calling 'error' will return the error string. template <typename T> class Try { public: static Try<T> some(const T& t) { - return Try<T>(SOME, new T(t)); + return Try<T>(t); } static Try<T> error(const std::string& message) { - return Try<T>(ERROR, NULL, message); + return Try<T>(Error(message)); } - Try(const T& _t) - : state(SOME), t(new T(_t)) {} + Try(const T& t) + : data(Some(t)) {} template <typename U> Try(const U& u) - : state(SOME), t(new T(u)) {} + : data(Some(u)) {} Try(const Error& error) - : state(ERROR), t(NULL), message(error.message) {} + : message(error.message) {} Try(const ErrnoError& error) - : state(ERROR), t(NULL), message(error.message) {} - - Try(const Try<T>& that) - { - state = that.state; - if (that.t != NULL) { - t = new T(*that.t); - } else { - t = NULL; - } - message = that.message; - } + : message(error.message) {} // TODO(bmahler): Add move constructor. - ~Try() - { - delete t; - } - - Try<T>& operator = (const Try<T>& that) - { - if (this != &that) { - delete t; - state = that.state; - if (that.t != NULL) { - t = new T(*that.t); - } else { - t = NULL; - } - message = that.message; - } + // We don't need to implement these because we are leveraging + // Option<T>. + Try(const Try<T>& that) = default; + ~Try() = default; + Try<T>& operator = (const Try<T>& that) = default; - return *this; - } - - bool isSome() const { return state == SOME; } - bool isError() const { return state == ERROR; } + // 'isSome' and 'isError' are mutually exclusive. They correspond + // to the underlying state of the Option. + bool isSome() const { return data.isSome(); } + bool isError() const { return data.isNone(); } const T& get() const { - if (state != SOME) { + if (!data.isSome()) { ABORT("Try::get() but state == ERROR: " + message); } - return *t; + return data.get(); } - const std::string& error() const { assert(state == ERROR); return message; } + const std::string& error() const { assert(data.isNone()); return message; } private: - enum State { - SOME, - ERROR - }; - - Try(State _state, T* _t = NULL, const std::string& _message = "") - : state(_state), t(_t), message(_message) {} - - State state; - T* t; + // We leverage Option<T> to avoid dynamic allocation of T. This + // means that the storage for T will be included in this object + // (Try<T>). Since Option<T> keeps track of whether a value is + // stored, we just ask it when we want to know whether we are + // storing a value or an error. Another advantage of leveraging + // Option<T> is that it takes care of all the manual construction + // and destruction. This makes the code for Try<T> really simple! + Option<T> data; std::string message; }; http://git-wip-us.apache.org/repos/asf/mesos/blob/18479776/3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp index 4041dc4..1e5f449 100644 --- a/3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp +++ b/3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp @@ -24,6 +24,9 @@ TEST(Stout, Some) ASSERT_SOME(t1); EXPECT_SOME(t1.get()); EXPECT_EQ(42, t1.get().get()); + t1 = None(); + ASSERT_SOME(t1); + EXPECT_NONE(t1.get()); Try<Result<int> > t2 = Some(42); ASSERT_SOME(t2);
