Error handling and exceptions
Yes, this is yet another post in the internet talking about using exceptions
versus error returns. The topic has been flaming up at my workplace for quite
some time now, and I felt that writing a blog post about it during the week-end
would help me focus my thoughts and give me time to explain my point with the
due care. In case you didn't know, I'm against using exceptions for error
handling (maybe having spent many years working with Qt has had an effect on
this); that does not mean that I never write code using exceptions: I certainly
do my good share of try ... catch when dealing with third-party code
(including the STL), but you won't find a throw in my programs.
I'm not going to write here all the reasons why I refrain myself from implementing error handling using exceptions; I'd rather like to focus on the one I consider to be the major one, and which I rarely see being given the due weight in the debate.
And please note that this post is about C++ only; it may be that exception handling in other languages is designed in such a way that all my concerns are addressed (either by the language itself, or by common error handling policies).
Code safety
I was about to title this “Code readability”, but this is more about code verifiability, that is making sure that the code is correct and, ultimately, safe. As we all know, code is written once but read many times, and even if it's code you've written yourself, chances are that in a few weeks time you'll have forgotten several details about it; error cases and error handling are one typical thing that doesn't stick in our memory for long.
When I look at a small piece of code, such as the one that can fit into my screen, or which I can read from a merge request diff, I want to be able to ascertain that the code I'm looking at is correct. Let's look at some examples.
A throw-free project
I just made this up, so please bear with me if it doesn't make any sense. What
I want to show is that code like the above has very few fault risks, if found
in a project which bans throwing errors as exceptions: if we exclude
out-of-memory errors, that are generally not handled to let the
application crash (though you can always catch them if you like), the reader can
easily verify that this code is safe. Coding style policies and naming
conventions can guarantee that setMaximumSpeed() and setName() won't have a
return value that needs to be checked, and all other method calls either return
an error that our code is properly handling, or return some value. Of course,
by just looking at this piece of code we cannot know if the
engineTemperature() method has some other overloaded sibling which accepts
passing a reference to a boolean and which could be used to detect an error;
so, it may be that our code could be improved in that respect, if we had a look
at the header files for the Car class — but this does deny the fact that a
simple glance at this snippet tells us exactly what errors are handled and what
could be going wrong.
Let's look at this code instead:
If we continue on the assumption that we are working on a project which bans
throwing exceptions, we can immediately say that this code is not safe: we
don't know if the car successfully executed a lap on the track, and our process
will crash if boundingRectArea() is invoked on a null object.
Enter the exception
In a project where exceptions are actively used, the code from the second
snippet is not obviously wrong anymore: maybe executeLap() cannot throw any
exceptions, or, if does, the caller of this snippet is catching the exception?
In order to figure out whether this code is correct, I need to see the
declaration of the executeLap() method, and hope that there's a nice
noexcept in there; if there isn't, I have to look at its implementation, and
recursively descend through all the methods it calls — at which point the safest
attitude is just to assume that it can throw. But that's only half of the
story, because once I accept the fact that executeLap() can throw, I need to
check whether the exception is properly handled: I have to check the
implementation of all the callers of my method, and if I don't find a catch
there, I'll have to recursively walk up the tree of their callers.
And indeed even the first snippet, which looked so harmless when exception
throwing was banned, suddenly becomes not obviously correct anymore: what if
executeLap() or getPath() also throw an exception? You might say that it
would be quite a silly thing to do, and I'd certainly agree; but it may be that
indeed they don't throw any exceptions in their implementation, but some of the
methods they call does.
A compromise: catch early, catch often
The obvious solution to the above issue is having a policy of handling exceptions right away, and explicitly rethrowing them (or even better, rethrow a different, more appropriate exception) up the stack:
What I can tell from the above snippet is that the code is handling errors, and this is somehow a relief. I'm sure some of you would suggest using a more specific catch clause, but for the sake of this example let's assume that this one is fine.
(Quick note: the above example does not catch std::exception, because that
would also catch the std::bad_alloc exception which is typically thrown in
out-of-memory situations; my advice is not handle it at all, unless you know
what you are doing)
In real life, though, you might find that try-ing on a rather large block of
operations is not enough: suppose that the Car methods all emit the same
exception type, and that you need to handle them differently depending on
when they occur. Then you'd need to split up the try into smaller blocks,
and at that point your code won't look any cleaner than the equivalent code
which uses ifs on return values. Of course if you own the Car class you could
modify it to throw different exceptions, in order to keep more operations
inside the try block and have specific catches at the end.
The big catch (pun intended)
Even once you've refactored your methods to get the best out of exceptions (where "best" is highly subjective, but let's assume that it just means that you are happy with your exception-throwing code), there's something that still bothers me, and that's exactly the same thing that proponents of exceptions use as a “pro” in their argumentations: the business logic of your code gets separated from the error handling. You get a nice block of pure logic, not cluttered with error checking, and a catch section (which I call “the big catch”) where error cases are handled.
I really don't see how that makes the code any more readable or safe: sure, the
logic is not intertwined with error handling and might help focus on the
expected flow of the operations (though, really, I do not think that normal
brains have a problem skipping over if blocks), but that's hardly what I'm
interested in when I want to check that the code is correct. Most of program
errors and bugs lie in handling the edge cases and the abnormal situations, the
seldomly taken code paths, and that's where I need to focus my attention.
When I see code like this one, I need to mentally build a mapping of
“operationX() → possible exceptions” (which, unless exception naming is
making this obvious, requires me to look at the implementation of the
operationX() functions), and then mentally reconstruct the possible code
paths in case operationX() fails, for each line of the try block.
Not seeing the errors right there, right away makes the correctness verification harder, which in turns means that the code becomes less safe. It will make you focus on the best case scenario, while ignoring all those annoying edge cases — too bad that 90% of the bugs are there.
Reading through the ISO C++ propaganda FAQ
I've been given a link to the C++ FAQ about exceptions, and unfortunately I read it. While there isn't much to argue on the technical side of it, it also carries some misleading statements, which might be true in absolute terms but don't let you see the big picture by not mentioning all that you need to know (which is the fundamental technique behind propaganda). An example is when they mention that eliminating ifs makes for more robust code, without mentioning that the same applies to all code branches, including exceptions.
Another argument that bothered me when I read it is the one about error propagation; this is the example they make:
The claim is that this code is more readable than the one with explicit error
handling, because all the f2(), f3, …, f9() functions don't have to
handle the error occurring in f10(). It is indeed a convincing argument,
when presented in these terms, but is this really how our code looks like? In
real life, you'll hardly have a chain of 1-liner functions, all defined next
to each other in the same file. The moment that you realize that each one of
these fn() functions might be twenty or thirty
lines long, and that they might be scattered over different files, and be
called not just by fn-1() but by any other
function in the codebase, the picture does not look so rosy anymore: we get
back to my main point of pain, that is that looking at the code of, say,
f5(), I will not be able to tell if the errors thrown by it, or by any of the
methods invoked by it, are properly handled.
Exceptions in APIs
A side note about projects using exceptions. I'm not really bothered when a library I need to use is throwing exceptions: having to write
is not less readable or less safe than the code I'd write if Foo::fetch()
returned an error code. I still do have a little complaint, because the library
author has given himself the right to decide that a failure in his library
should be considered a critical fault, whereas it may be that in my program it
is an expected failure and using exceptions imposes a penalty which could have
been avoided. But I digress.
As long as the library documents which exceptions are thrown, it is used by many
people (which hopefully means that it has few bugs) and it is a library that I
don't need to contribute to, wrapping some of its methods in try blocks is
something I can live with.
One situation where I actually wish that libraries threw an exception is in
out-of-memory situations; in that case, of course, I'd expect them to throw
nothing else than std::bad_alloc, which is the exception emitted by the
standard library in such situations. That allows the caller to decide whether
to ignore the exception and have the process terminated (which is what I
usually do, at least in desktop applications) or try their luck and handle the
failure — the latter is not easy, but it can certainly be done.
This is one case where error returns can be problematic, because it's likely that your code would look something like
|
if (!Foo::open(fileName)) { // suppose that this returns Error::OutOfMemory
|
|
log("Failed to open " << fileName);
|
|
return false;
|
|
}
|
and in this case there's actually a risk that your code is going to trigger an out-of-memory error in logging the message; this shouldn't be a concern in most cases, but I can imagine some situations where one might want to know which was the exact operation that first incurred in the out-of-memory failure.
So, I'm actually fine with new throwing. As for my code, my throw statement
is actually spelt as return.