I'm going back and forth between the two versions. I think I'd take the 2nd (the custom class), though maybe with the API I had used in my example instead (i.e, msg.failed() and then a single status() method for both cases) as having two methods error/status returning the same thing looks a bit odd. But not a big deal either way, any of these options sounds fine to me.
Robin On Fri, Jan 06, 2017 at 10:32 -0800, you wrote: > > I think the name "error" is not just misleading but would also turn > > out tricky to use correctly. > > Agreed. > > > auto msg = ep.receive(); > > > > if (msg) > > return f(*msg); // unbox contained message > > > > if (msg.failed()) > > cout << "Trouble: " << to_string(msg.status()) << endl; > > else > > cout << "Status change: " << to_string(msg.status()) << endl; > > > > In either case one could then also use a switch to differentiate the > > status() further. > > I think this is semantically what we want. Because broker::error is just > a type alias for caf::error (and broker::expected just caf::expected), > it's currently not possible to change the API of those classes. I see > two solutions, one based on the existing vehicles and one that > introduces a new structure with three states for T, error, and status. > > Here's the first. A caf::error class has three interesting member > functions: > > class error { > uint8_t code(); > atom_value category(); > const message& context(); > }; > > The member category() returns the type of error. In Broker, this is > always "broker". CAF errors have "caf" as category. We could simply > split the "broker" error category into "broker-status" and > "broker-error" to distinguish the error class. We would keep the two > different enums and provide free functions for a user-friendly API. > Example: > > // -- Usage ------------------------------------------- > > auto msg = ep.receive(); // expected<message> > > if (msg) { > f(*msg); // unbox message > } else if (is_error(msg)) { > cout << "error: " << to_string(msg.error()) << endl; > if (msg == error::type_clash) > // dispatch concrete error > } else if (is_status(msg)) { > cout << "status: " << to_string(msg.error()) << endl; > if (msg == status::peer_added) > // dispatch concrete status > } else { > // CAF error > } > > // -- Broker implementation --------------------------- > > using failure = caf::error; > > enum status : uint8_t { /* define status codes * /}; > > enum error : uint8_t { /* define error codes */ }; > > bool is_error(const failure& f) { > return f.category() == atom("broker-error"); > } > > bool is_status(const failure& f) { > return f.category() == atom("broker-status"); > } > > template <class T> > bool failed(const expected<T>& x) { > return !x && failed(x.error()); > } > > template <class T> > bool operator==(const expected<T>& x, status s) { > return !x && x.error() == s; > } > > The only downside here is that we're still calling msg.error() in the > case of a status. That's where the second option comes in. Let's call it > result<T> for now (it won't matter much, because most people will use it > with "auto"). A result<T> wraps an expected<T> and provides a nicer API > to distinguish errors and status more cleanly. Example: > > // -- Usage ------------------------------------------- > > auto msg = ep.receive(); // result<message> > > if (msg) { > f(*msg); // unbox T > } else if (auto e = msg.error()) { > cout << "error: " << to_string(*e) << endl; > if (*e == error::type_clash) > // dispatch concrete error > } else if (auto s = msg.status()) { > cout << "status: " << to_string(*s) << endl; > if (*s == status::peer_added) > // dispatch concrete status > } else { > assert(!"not possible"); > } > > // -- Broker implementation --------------------------- > > enum status : uint8_t { /* define status codes * /}; > > enum error : uint8_t { /* define error codes */ }; > > template <class T> > class result { > public: > optional<caf::error> status() const; > optional<caf::error> error() const; > > // Box semantics > explicit operator bool() const; > T& operator*(); > const T& operator*() const; > T* operator->(); > const T* operator->() const; > > private: > expected<T> x_; > }; > > Thoughts? > > Matthias > -- Robin Sommer * ICSI/LBNL * ro...@icir.org * www.icir.org/robin _______________________________________________ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev