Reference-counted error objects

[Gregory]

Hi,

One of the disadvantages of using error handling with error codes
instead of exception handling is that error codes retuned from a
function can be forgotten to check thus leading to
unexpected run-time problems. Exceptions on the other hand can not be
simply ignored.
In order to solve this error codes problem I suggest using error object
instead of error codes.
Each function has to return an error object that stores an error
condition (true/false) that must be eventually tested, otherwise
warning/exit/exception (may be not a good idea) will be generated.
By eventually I mean that it must not be the same instance of the error
object that has to be tested, it can also be one of its saved copies
(copy-constructed or assigned w/ operator=) that are passed along the
program flow.

Such error object may be implemented using internal reference counter
which tracks all its copies and if none of the copies was tested the
last copy destructor signalizes the problem.

class Error
{
public:
Error(bool hasErr, const string& file, int line);

~Error()
{
if (! m_refCntError->m_wasChecked)
{
cerr << "WARNING: Error at file " <<
m_refCntError->m_file << ", line " <<
m_refCntError->m_line <<
" has been never checked !" << endl;
}
}

bool hasError()
{
// Public member for simplicity
m_refCntError->m_wasChecked = true;

return m_refCntError->hasError();
}

// Reference-counted implementation
Error(const Error&);
Error& operator=(const Error&);
.......
private:
RefCntError *m_refCntError;
};

If none of the error object copies called hasError() method the warning
is issued.

There is obvious performance penalty using error objects as opposed to
the error codes.
So it can be only used in the debug mode.

Does this strategy make sense ?

Gregory

 

[Greg]

Hi,

One of the disadvantages of using error handling with error codes
instead of exception handling is that error codes retuned from a
function can be forgotten to check thus leading to
unexpected run-time problems. Exceptions on the other hand can not be
simply ignored.
In order to solve this error codes problem I suggest using error object
instead of error codes.
Each function has to return an error object that stores an error
condition (true/false) that must be eventually tested, otherwise
warning/exit/exception (may be not a good idea) will be generated.
By eventually I mean that it must not be the same instance of the error
object that has to be tested, it can also be one of its saved copies
(copy-constructed or assigned w/ operator=) that are passed along the
program flow.

Such error object may be implemented using internal reference counter
which tracks all its copies and if none of the copies was tested the
last copy destructor signalizes the problem.

class Error
{
public:
Error(bool hasErr, const string& file, int line);

~Error()
{
if (! m_refCntError->m_wasChecked)
{
cerr << "WARNING: Error at file " <<
m_refCntError->m_file << ", line " <<
m_refCntError->m_line <<
" has been never checked !" << endl;
}
}

bool hasError()
{
// Public member for simplicity
m_refCntError->m_wasChecked = true;

return m_refCntError->hasError();
}

// Reference-counted implementation
Error(const Error&);
Error& operator=(const Error&);
.......
private:
RefCntError *m_refCntError;
};

If none of the error object copies called hasError() method the warning
is issued.

There is obvious performance penalty using error objects as opposed to
the error codes.
So it can be only used in the debug mode.

Does this strategy make sense ?

Yes, a class to enforce error checking does make sense, particularly if
the program has some reason not to adopt exceptions for error handling.

I do have a few suggestions for simplifying and streamlining the Error
class:

For reasons of efficiency, the Error class should not be any larger
than the error code itself. If it were the same size as an error code,
then Error objects could be passed and returned by value fairly
efficiently.

Along these lines, it is not really necessary to ref count Error
instances. A simpler approach I think would be to have each error
result have only one Error object "owner" at a time. An Error object
that owns a result must either be checked or have its error result
copied to another Error object, before it is destroyed. In a way, an
error result something of a "hot potato", the routine that receives it
must either deal with it or pass it up the calling chain. But
eventually some code somewhere will have to check it.

Lastly, there should be as little difference as possible in how errors
are handled between the debug and production builds. After all, the
software that is shipped should be as close as possible to the software
that was tested. A more efficient Error class could be used in a
non-debug builds, with the only difference being that asserts become
no-ops.

To illustrate how these suggestions might be implemented, I have
written an Error class (below). An Error object is the same size as an
error code (an int in this example). An Error object, once assigned an
error result, becomes the "owner" of that result. The program must
ensure that an error result is either read or copied to another Error
object before the Error object owner is destroyed. A few test cases are
provided to give a better idea of how this all works.

#include <assert.h>

class Error
{
mutable int mErrorCode;

// the value of kErrorCodeChecked should not conflict
// with any actual error code

static const int kErrorCodeChecked = -1;

void ClearError() const
{
mErrorCode = kErrorCodeChecked;
}

public:
Error( int err = kErrorCodeChecked)
: mErrorCode( err)
{
}

Error( const Error& rhs)
: mErrorCode( rhs.GetErrorCode())
{
}

~Error()
{
assert( mErrorCode == kErrorCodeChecked);
}

int GetErrorCode() const
{
int err = mErrorCode;
ClearError();
return err;
}

Error& operator=(int err)
{
assert(mErrorCode == kErrorCodeChecked);

mErrorCode = err;
return *this;
}

Error& operator=(const Error& rhs)
{
assert(mErrorCode == kErrorCodeChecked);

mErrorCode = rhs.GetErrorCode();
return *this;
}
};


// Test Cases

// a routine that returns an error
Error ReturnsError()
{
int a = 5 * rand();

return -10;
}

// Fails - Error code discarded
Error
TestFailureOne()
{
ReturnsError();
return 0;
}

// Fails - Error code stored but not retrieved
Error
TestFailureTwo()
{
Error err = ReturnsError();

return 0;
}

// OK - Error code is checked
Error
TestCaseOne()
{
Error err = ReturnsError();
int err = err.GetErrorCode();

return 0;
}

// OK - Error code is returned to caller
Error
TestCaseTwo()
{
Error err = ReturnsError();

return err;
}

int main()
{
TestFailureOne();
TestFailureTwo();
TestCaseOne();
TestCaseTwo();
}

Greg

 

[Gregory]

Greg, thanks ! You comments are very insightful. I like
your "hot potato" metaphore Indeed I do not see any need in the
tracking of
all error object copies all together (using ref-conting). At least I
can't find an
example justifying these efforts. Yes, "single ownership" is a proper
strategy.
However it seems better to separate mErrorCode two responsibilities of
storing error
code and indicating never-checked error.
Here is an example of such necessity:

Error Test()
{
Error err = ReturnsError();

if (err.GetErrorCode())
{
// err already does not contain original error code
return err;
}

return 0;
}

Returned 'err' already erased its original error code. If we separated
the above mentioned responsibities, then the error code check flag
would be marked and the error code
would be left unchanged. We could add additional mErrorCheck private
data memeber to the Error class and treat it and mErrorCode separately.

By the way all four test functions in your main() will be asserted
right after they return (returned error is not checked).

Another issue is in the following example:

Error test()
{
Error err1 = ReturnsError();
Error err2 = ReturnsError();
Error err3 = ReturnsError();

if (err1.GetErrorCode() || err2.GetErrorCode() ||
err3.GetErrorCode())
{
return -1; // return error
}
}

Using lazy evaluation of OR statement neither the second nor the third
error might
will be checked. My code also suffers from this problem.
May be it's just bad style to leave error unchecked for some time, may
be not. But I don't
se appropriate solution for this problem and this code is sometimes
more clear and easy to write.

Regards,

Gregory