[Bro-Dev] Rethinking Broker's blocking API

[Robin Sommer]

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 * robin at icir.org * www.icir.org/robin