Thread: So... handling errors.

Ok. It came the time I have to brace myself and start delineating some error handling strategy for my pet project. I'm probably doing this late already since the usual advise is "delineate a rational error handling strategy early in your project and stick to it."

But I understand the operative word in the above quote is rational. And that is what scares me. Everyone I read so far are very clear that there is no clear way to deal with this. Freedom can be a very scary thing, and it doesn't mean we can't go wrong. Probably the best quote I've found so far is "There are two ways to write error-free programs; only the third one works".

So, I won't be asking for specific advise. I understand you can't give it unless I can give a specific example. Which unfortunately for most cases, I can't.

Anyways, the tools I'm going to use are:
* C++ Exceptions,
* Assert,
* Boost::static_assert (compile-time assertions)
* and of course... avoiding errors.

Some notions:
- My project is OOP oriented. Everything is an object. Objects have relations.
- Classes responsibility includes managing themselves.
- Some preconditions, postconditions and invariants are dependant on the programmer, others on user input.

And here goes my first doubt (more to come):
One of the difficulties I'm having is understanding exactly when and how to use exceptions. Here's an example, some classes are instantiated from dat files. If the file is non existing or its data is corrupt this is a runtime error of which there is no solution. The program must abort. Should I throw an exception from within the class even considering that the only way to handle is to abort? And how do I abort gracefully from within a class definition making sure all my resources are cleaned?

Currently what I have is something like this:

Code:

// CObject.hpp

class objects_file_error : public std::runtime_error {
public:
    objects_file_error(): std::runtime_error("obj.dat file is missing or corrupt.") {}
};

class CObject {
// inlining it just to cut down on typing
virtual bool load(const std::string& id) try {
    /*...*/
} catch (objects_file_error& err) {
    /*... here I output the error message ... */
    throw;
}
};

Is this correct?

And what about moving user-defined exception classes to their own header file and create some hierarchy from there?

I'm sorry for bumping this. But it got moved way down when the forums date was fixed.

For any exception that should completely terminate the program without any hope of recovery, just let it propagate all the way back to main(), pop up an error message in the catch(), and return an error code (e.g. anything nonzero).

> Here's an example, some classes are instantiated from dat files.
Are these files loaded as the result of users selecting a file?
If so, then you should return to the point where the user made the choice.
If the file was specified on the command line, then exit with an error message.
If the file was specified using a "File load", then popup an error message and wait for the next user request.

>Should I throw an exception from within the class even considering that the only way to handle is to abort?
Yes. Always let the main program decide how to handle an error. Even if you "know for a fact" that the only solution is to abort, you could still be wrong. Also, keeping termination capabilities at roughly the same tier aids in transparency. Finally, you should refrain from printing error messages within your class as well. Both reporting and reacting to exceptions is the job of the controlling program, not the classes that throw the exceptions.

>And how do I abort gracefully from within a class definition making sure all my resources are cleaned?
You don't. Clean up the resources that the class uses and pass on the exception to the highest level possible. You never know, a level higher up may be able to successfully recover.

Ok. Most excellent. Thanks everyone. All have been very helpful. I think I'm getting there...

In short, I should change my member function declaration to:

virtual bool load(const std::string&) throw(objects_file_error);

I should avoid outputting an error message considering I may want to reuse this header in a project/place where there may be in fact some recoverable/reporting procedure the user of this class wants to implement.

Here's one more question:

- I'm using assert liberaly to report, mainly, preconditions at function level. On all cases, derived from programming errors. However, even that line seems tricky sometimes. Is the programmer doing an error because he was distracted, or because he doesn't know any better? In other words because my class doesn't inform him of those preconditions. (or worse, because it doesn't enforce them when it should). The example is this:

CAttribs is a class holding a single data member. An unsigned long representing a packed value of the fighter attributes. Str, Dex, Con, Int, Smartness and Sanity. Each attribute can only be in the range [1,20] (5 bits). 6 attributes x 5 bits = 30 bits.

The most shocking example is not within the class itself but on an helper function I declared inside the same namespace as that of the class.

Code:

// generates random attributes within a range.
// Returns the packed value to be used in CAttribs(unsigned long) constructor
unsigned long attributes::RandomAttrib(unsigned int lo, unsigned int hi);

Inside the funcion definition I assert lo and hi to be >0 and <21, respectively. I also assert lo < hi. The last assert is a no brainer. Good on me. But what about checking for the range?

Am I doing the right thing leaving it all to an assert? Or should my class enforce this rule?

>virtual bool load(const std::string&) throw(objects_file_error);
Don't waste your time with exception specifications. They're more trouble than they're worth.

>Am I doing the right thing leaving it all to an assert? Or should my class enforce this rule?
I think an assert is fine. Presumably you're documenting what the function expects, so failure to meet those expectations is a breach of contract on the part of the calling code. Your class could enforce it, but then you bring up questions of how to enforce it.

I thought of making it generic. A template with two nontype arguments representing the range. However, this would bring problems if anything higher than 31 was passed as an argument for the high range. At that point 6 bits would be needed for each attribute. Voil&#225;! Exception handling.

Another possibility was just one nontype argument and two specializations of 32 and 64 mapping to unsigned long and double, respectively. This would already provide an interesting range of possible values. But... then I would be solving nothing. Range checking would still have to be done probably by asserts just like now.

Not that I have any interest in making this kind of object generic. Just what occured to me when creating it.

Anyways, back to earth, thanks Prelude. Assert it is.

That's what BOOST_STATIC_ASSERT is for.
So the end result could look like this:

Code:

template <unsigned int LOW_BOUND, unsigned int HI_BOUND>
unsigned long attributes::RandomAttrib(unsigned int lo, unsigned int hi)
{
  BOOST_STATIC_ASSERT(LOW_BOUND < HI_BOUND);
  BOOST_STATIC_ASSERT(HI_BOUND < (1 << BITS_PER_ATTRIBUTE));
  assert(lo >= LOW_BOUND);
  assert(lo < hi);
  assert(hi <= HI_BOUND);
}

All in all, I think it's a waste of time. Document that the values lo and hi must be between 0 and (1 << BITS_PER_ATTRIBUTE) and assert against those only. The template arguments add nothing.