I have an API written in C, which produces a result by returning a pointer to allocated memory.
For using it with C++ (C++11) I've wrapped the function calls in objects, which keep the result in a std::shared_ptr
. So far so good.
However, the C library features two functions for every operation. One produces possibly an error, the other never. Let's call them
some_pod * do_it_with_error(Parameter ..., Error **)
and
some_pod * do_it_without_error(Parameter ...)
I can pass in the address of an Error *
pointer to the first function and if there's an error it won't be NULL
afterwards.
For solving this I thought of two different implementations.
First, I could SFINAE for choosing between the with_error
and without_error
functions, like so:
template<typename NoThrow = void>
struct do_it {
public:
void operator()(...)
{
Error * error = NULL;
m_result = std::shared_ptr<some_pod>(do_it_with_error(..., &error));
if (error) {
throw(MyExceptionWithError(error));
}
}
private:
std::shared_ptr<some_pod> m_result;
};
template<>
class do_it<std::nothrow_t> {
public:
void operator()(...) noexcept
{
if (! m_result) {
m_result = std::shared_ptr<some_pod>(do_it_without_error(...));
}
}
private:
std::shared_ptr<some_pod> m_result;
};
Usage:
do_it<> di_error;
try {
di_error(...); // might throw
} catch (...) {}
do_it<std::nothrow_t> di_no_error;
di_no_error(...); // won't throw for sure
However, since the result is wrapped in a std::shared_ptr
there will also be
a get()
method:
const std::shared_ptr<some_pod> & get(void) { return m_result; }
For error checking I could just implement a another method check
.
The default implementation would now look like this:
struct do_it {
public:
// will never throw
const std::shared_ptr<some_pod> &
get(void) noexcept
{
if (! m_result) {
m_result = std::shared_ptr<some_pod>(do_it_without_error(...));
}
return m_result;
}
// might throw
do_it &
check(void)
{
if (! m_result) {
Error * error = NULL;
m_result = std::shared_ptr<some_pod>(do_it_with_error(..., &error));
if (error) {
throw ...;
}
}
return *this;
}
private:
std::shared_ptr<some_pod> m_result;
};
Usage:
do_it di_error;
try {
auto result = di_error.check().get();
} catch (...) {}
do_it di_no_error;
di_no_error.get();
So, both implementations seem to be equally good (or bad) to me.
How could I decide which one to use.
What are the Pros and Cons?
3 Answers 3
Too complicated - wrap them both as if either could throw an error, that the 2nd fn will never throw only means the compiler will never have to call the error handling routines (and might even optimise it out entirely) - either way your code will perform just as well thanks to C++s design.
Your code will also be much easier to understand, even with this redundant error handling (that one day might be used for other, unexpected errors)
-
Are both solutions too complicated? Do you mean to wrap them in two separate classes? Like
struct do_it_with_error
anddo_it_without_error
?jchnkl– jchnkl2014年02月05日 11:17:01 +00:00Commented Feb 5, 2014 at 11:17 -
wrap them as if they were not different things, one will throw exceptions the other will not - the caller wouldn't have to know which type of API call he was calling as the wrapper class would look identical regardless of which type of underlying API it was calling. I may have made as assumption based on your question but I don't know how these functions are actually organised, how many of them there are, if they are generated, etc. But a single interface for the caller is best so he wouldn't need to call check().get() for some and get() for others.gbjbaanb– gbjbaanb2014年02月05日 11:26:28 +00:00Commented Feb 5, 2014 at 11:26
-
I see. The C API is generally organized in
*_with_error
and*_without_error
. The*_without_error
won't immediately produce an error, but queue it up in the event queue which belongs to the library. So, for the user it is important to know if he's calling an API function which generates an error immediately or deferred. I think that also the reason why I cannot easily wrap those functions into one method. A single interface would of course be the best solution, but I can't think of a way how to do that here.jchnkl– jchnkl2014年02月05日 11:38:19 +00:00Commented Feb 5, 2014 at 11:38 -
1Then I'd create 2 differently named wrappers - do_it_now and do_it_deferred (or whatever) so its really obvious, or if the user could call one or the other (ie he chooses what type of handling he wants) pass in a handle to the deferral queue reader object (or suchlike) to distinguish between the wrapper objects. Currently the 2nd option means he has to know there's a check() method to call, and he might forget or otherwise not bother calling it. That'd be bad.gbjbaanb– gbjbaanb2014年02月05日 11:53:40 +00:00Commented Feb 5, 2014 at 11:53
-
Ok, I see your point here. Providing two different wrappers would mirror the C API. The check() method would obfuscate things a bit more. What about the first solution using SFINAE? That'd be just like two different method calls, but by using a template parameter.jchnkl– jchnkl2014年02月05日 12:28:31 +00:00Commented Feb 5, 2014 at 12:28
In C the error objects are somewhat pain to handle, so there is the other overload that stores the errors somewhere. But in C++ exceptions make it all much easier to handle. So I would only wrap the variant that reports errors and converted the errors to exceptions and ignore that the other API even exists.
-
Sorry, I missed out a bit of information. I've added it above. Unfortunately I cannot just leave out a part of the functionality.jchnkl– jchnkl2014年02月05日 12:29:43 +00:00Commented Feb 5, 2014 at 12:29
-
@jrk: Then simple separate wrappers are probably the best way to go. The user needs to know how to handle the errors and has to not get it wrong by mistake.Jan Hudec– Jan Hudec2014年02月05日 12:37:01 +00:00Commented Feb 5, 2014 at 12:37
-
Thanks for your answer. I'd accept it too, if I could accept two answers. :) You and gbjbaanb brought me on the right track!jchnkl– jchnkl2014年02月05日 19:12:51 +00:00Commented Feb 5, 2014 at 19:12
For each operation, you have three distinct subjects to take care about, say, workload, result and error handling. Let's see them all in isolation:
Workload: shouldn't be duplicated. Always call the C function possibly producing error information, as it's the most complete of each pair.
Result: PODs can be kept as they are in the C API and hold/retrieved through
shared_ptr
objects, but this is only one of many possible options -if, certainly, a quite straightforward one. Maybe some of this PODs are worth to be wrapped into, or converted to, copy-constructible/move-constructible classes and directly returned from theoperator()
call.Error handling: the way your C++ function objects do (or do not) communicate error-related information is seconday and orthogonal to the main workload. Ideally, you want to be able to change that error handling mechanism depending on the context, without affecting the workload itself. So view it as a dependency that you inject on the function object at your will, either at construction time or at calling time. This way, you can add additional error handling mechanisms in the future, other than current do-nothing and exception-throwing ones (what if you do end wanting to notify errors thorugh console messages, for example?).
I'll end providing a simple implementation that could serve as example:
class DoSomeStuff
{
public:
class Result
{
public:
Result() : _result() {}
Result(const Result &rhs) : _result(rhs) {}
~Result() {}
Result &operator=(const Result &rhs)
{
if(&rhs != this)
_result = rhs._result;
return *this;
}
int GetSomeData() const { return _result->_some_int_value; }
private:
shared_ptr<some_pod> _result;
};
struct IErrorHandler
{
virtual void NotifyError(Error &) = 0;
};
Result operator()(/*params*/, IErrorHandler &errorHandler)
{
Error *error = { nullptr };
shared_ptr<some_pod_type> result = { do_some_stuff_with_error(/*params*/, &error) };
if(error != nullptr)
errorHandler.NotifyError(*error);
return Result(result);
}
};
Note that Result and IErrorHandler don't have to be included inside the function class; I've done this way because they're both dependencies, but can be extracted in order to avoid duplication among several function classes.
shared_ptr
! If it's a C API, the object it returns was allocated withmalloc
and therefore must be deleted withfree
. If you justdelete
is asshared_ptr
does by default, it will explode on some platforms.shared_ptr
can get a custom deleter, pass it one that releases usingfree
.