ecorm / cppwamp

C++ client library for the WAMP protocol.
Boost Software License 1.0
35 stars 9 forks source link

Error code handling with coroutines #53

Closed taion closed 9 years ago

taion commented 9 years ago

The documentation recommends handling this by passing in a pointer: https://github.com/ecorm/cppwamp/wiki/Error-Handling#passing-an-error-code-pointer

However, the idiomatic way to do so is to use the overloaded operator[] on the yield_context. Does this usage not work? I can't currently build the tests because I'm using clang instead of gcc, but am not going to try to fix the build errors until the next release lands.

ecorm commented 9 years ago

I had already considered trying to use operator[]. See http://stackoverflow.com/questions/27972851

There is one major problem: basic_yield_context::operator[] takes a boost::system::error_code, whereas CppWAMP exposes only std::error_code to the user (it converts Asio error codes into std error codes when propagating them back to the user).

ecorm commented 9 years ago

An alternative to operator[] would be to provide this non-member overload:

YieldWrapper<H> operator+(const boost::basic_yield_context<H>&, std::error_code&)

where YieldWrapper bundles together a boost::basic_yield_context with a std::error_code*.

It would be used like this: session->join(Realm("foo"), yield + ec);

But the way I have it now with: session->join(Realm("foo"), yield, &ec); makes the reference documentation much easier to write (and understand). I know that this idiom is used in Qt for returning optional out parameters.

If I'm not able to use the operator[] idiom used by Asio, then I figure I might as well use the optional out parameter pointer idiom used by Qt.

taion commented 9 years ago

Fair enough. Probably not worth switching to non-Boost Asio just to use std::error_code instead of boost::system::error_code. It is a little unfortunate, though.

ecorm commented 9 years ago

The standalone Asio library does indeed use std::error_code. But since CppWAMP already depends on Boost.Coroutine, it makes sense for it to depend on Boost.Asio as well.