| Date: | April 22, 2004 / year-entry #155 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20040422-00/?p=39683 |
| Comments: | 84 |
| Summary: | Just because you can't see the error path doesn't mean it doesn't exist. Here's a snippet from a book on C# programming, taken from the chapter on how great exceptions are. try { AccessDatabase accessDb = new AccessDatabase(); accessDb.GenerateDatabase(); } catch (Exception e) { // Inspect caught exception } public void GenerateDatabase() { CreatePhysicalDatabase(); CreateTables();... |
Just because you can't see the error path doesn't mean it doesn't exist. Here's a snippet from a book on C# programming, taken from the chapter on how great exceptions are.
Cleaner, more elegant, and wrong. Suppose an exception is thrown during CreateIndexes(). The GenerateDatabase() function doesn't catch it, so the error is thrown back out to the caller, where it is caught. But when the exception left GenerateDatabase(), important information was lost: The state of the database creation. The code that catches the exception doesn't know which step in database creation failed. Does it need to delete the indexes? Does it need to delete the tables? Does it need to delete the physical database? It doesn't know. So if there is a problem creating CreateIndexes(), you leak a physical database file and a table forever. (Since these are presumably files on disk, they hang around indefinitely.) Writing correct code in the exception-throwing model is in a sense harder than in an error-code model, since anything can fail, and you have to be ready for it. In an error-code model, it's obvious when you have to check for errors: When you get an error code. In an exception model, you just have to know that errors can occur anywhere. In other words, in an error-code model, it is obvious when somebody failed to handle an error: They didn't check the error code. But in an exception-throwing model, it is not obvious from looking at the code whether somebody handled the error, since the error is not explicit. Consider the following:
Guy AddNewGuy(string name)
{
Guy guy = new Guy(name);
AddToLeague(guy);
guy.Team = ChooseRandomTeam();
return guy;
}
This function creates a new Guy, adds him to the league, and assigns him to a team randomly. How can this be simpler? Remember: Every line is a possible error.
When you're writing code, do you think about what the consequences of an exception would be if it were raised by each line of code? You have to do this if you intend to write correct code. Okay, so how to fix this? Reorder the operations.
Guy AddNewGuy(string name)
{
Guy guy = new Guy(name);
guy.Team = ChooseRandomTeam();
AddToLeague(guy);
return guy;
}
This seemingly insignificant change has a big effect on error recovery. By delaying the commitment of the data (adding the guy to the league), any exceptions taken during the construction of the guy do not have any lasting effect. All that happens is that a partly-constructed guy gets abandoned and eventually gets cleaned up by GC. General design principle: Don't commit data until they are ready. Of course, this example was rather simple since the steps in setting up the guy had no side-effects. If something went wrong during set-up, we could just abandon the guy and let the GC handle the cleanup. In the real world, things are a lot messier. Consider the following:
Guy AddNewGuy(string name)
{
Guy guy = new Guy(name);
guy.Team = ChooseRandomTeam();
guy.Team.Add(guy);
AddToLeague(guy);
return guy;
}
This does the same thing as our corrected function, except that somebody decided that it would be more efficient if each team kept a list of members, so you have to add yourself to the team you intend to join. What consequences does this have on the function's correctness? |
Comments (84)
Comments are closed. |
Another general design principle: use Finally, and never leave a function without the data being in a consistent state.
In the last example the problem is if an exception occurs during AddToLeague(guy) you have a guy on a team but he doesn’t belong to the league. As in the previous post, use a finally to remove guy from the team before leaving.
I’m not familiar with C#, but I know that with C++, have a properly written destructor should take care of lots of these problems. In the examples above, for instance, the Guy object’s destructor should remove him from any leagues and/or teams to which he belongs. Well-written destructors go a long way toward the production of exception-safe code.
All you did was push the problem somewhere else. In C++ the problem is knowing when to call "delete" (and therefore cause the destructor to run).
Sorry:
public class Guy
{
public LeagueTeam Team
{
set
{
try
{
value.Add(this);
_team = value;
}
catch (Exception ex)
{
// rollback any possible effect
if (null != value && value.Contains(this))
{
value.Remove(this);
_team = null;
}
}
}
get
{
return _team;
}
}
}
You forgot to rethrow the exception. And this only fixes the setting of the team. Adding to the league is still an exception point.
Do a Google search for Dave Abraham’s exception safety guarantees.
Dave is clearest thinkers on exception safety by far.
Sometimes you can emply the "strong guarantee" which says that everything is transactional and that all functions should undo what they did if an exception is thrown.
Sometimes that’s not possible ("how do you unfire a booster rocket?") or not practical (performance) to have the strong guarantee, and then you have to go for the basic guarantee.
And why do I have to type these longish comments inside this little edit box?
Dejan
Misread — league. Gotcha.
This "worst practice" of designing bad error handling has security impacts as well. Imagine if instead of CreateDatabase(); CreateTable(); it was CreateUser(); CreateRandomPassword(); ! You don’t want to get into situations where you create users with blank passwords and then die horribly.
Or, worse, DebitCheckingAccount(); CreditSavingsAccount();
Any time sensitive data that must be consistent is manipulated, you’ve got to get the error handling right.
Quite right. Fogot to rethrow, but if you wrap the caught exception, how is the addition to the team a problem? In other words,
public class Guy
{
public LeagueTeam Team
{
set
{
try
{
value.Add(this);
_team = value;
}
catch (Exception ex)
{
// rollback any possible effect
if (null != value && value.Contains(this))
{
value.Remove(this);
_team = null;
}
throw new ApplicationException("Could not join guy to league.", ex);
}
}
get
{
return _team;
}
}
}
The application is in a consistent state because the guy’s team isn’t set and the team doesn’t have the guy. Am I missing something?
But if an exception is thrown adding the guy to the league, you now have a team with a guy that doesn’t belong to the league.
Team should handle adding the guy to the league as well inside the Add(Guy guy) so that when you join one, you join the other.
That works if there is only one thing to be done outside adding to the league (picking a team). But what if there are several things you need to do, like say, assign the guy a membership number and adding to the orientation meeting.
Not sure where you’re going with this, we could add requirements ad nauseum. :) How would you handle this?
I think the underlying point here is that exceptions don’t really make handling errors correctly any easier. As amply demonstrated by the multiple incorrect attempts to correct Raymond’s example.
All this is why I’m a big fan of explicit handling and error codes.
Right, my point is that it isn’t any easier. It’s different. And in my opinion, harder since the error handling is much more subtle.
Christian: Specifically, I was pointing out that your solution wasn’t generalizable. It solved the immediate problem but wasn’t agile.
I’d add that in addition to the fact that these calls can fail due to the obvious reasons you also need to consider Thread.Abort() and Out Of Memory in managed code.
Imagine if you’re doing:
try
{
MayFail();
}
finally
{
RollbackState();
}
and RollbackState hasn’t been JITed yet! If MayFail failed due to OOM then the call to RollbackState may fail as well.
And Thread.Abort can happen pretty much anywhere so you won’t know what your state was like when you finally handle the exception.
I was doing the simplest thing that could possibly work. Generalization can happen during refactoring. I get the distinct feeling I need a Zen slap because I fail to see the point. I’m clear on the fact that every line of code is a potential exception waiting to happen, but the critiques are still too vague for me. Could someone point out a generalizable/"good" solution? I have yet to see one….
Some of this is covered in Sutter’s "Exceptional C++" book, which covers exception handling while building a C++ stack implementation. Dave Abrahams’ reasoning about exceptions are included, which includes some rules of thumb about order of operations (operations that throw versus operations that change state, etc.)
I think Raymond is not saying you can’t fix the code (er… sorry about the double negative), he’s saying that exception handling doesn’t magically make it easier – often you have to be more careful to do it correctly so you aren’t dangling between valid states.
It’s good that setting the property did internal rollback, but the next layer out may also have to do rollback, so you can’t fix it entirely in the property itself.
I agree, but that’s a "higher-level" rollback. The setters just handle rollbacks associated with bi-directional associations (object state consistency). This is where Command objects with Undo support or transactions come into play (for higher-order consistency).
Raymond,
Can you show the right way to do it not using exceptions and using error codes instead?
Eric:
> Or, worse, DebitCheckingAccount(); CreditSavingsAccount();
>Any time sensitive data that must be consistent is manipulated, you’ve got to get the error handling right.
That code is usually written as:
DebitCheckingAccount(context);
CreditSavingsAccount(context);
commit(context);
If somebody terminates the thread which this is happening, then I suppose either of these scenario could happen:
1) this was the only thread in the process, so the process terminates too.
2) this was a worker-thread, and the main-thread can find out about this.
Nr. 1 is the easiest, IMHO. I think of something such as Outlook here. Suddenly the computer crashed while it was doing something. Next time you start it, Outlook sees that it was terminated and scans the mailbox-file for possible corruptions.
For nr. 2 the main-thread needs some way to find out that the worker-thread was terminated in an inconsistent state. If so, then it could (try) to clean up the mess left behind.
For the banking-example above[1], some transaction-mechanism should be there. Then the transaction manager can ‘just’ do a rollback.
But uh, yeah.. since I’ve started to write a book or something about transactions here now :P, IMO this clearly underlines Raymond’s point here :)
[1] http://blogs.msdn.com/oldnewthing/archive/2004/04/22/118161.aspx#118252
Writing complete and proper error handling code is difficult and requires lots of testing, it doesn’t matter if you use exceptions or error-code model.
I always suspected that exceptions were invented by programmers saying, "it’s so difficult to write proper error-handling code, so let’s invent something that magically traps all errors". Of course being able to trap them doesn’t mean error conditions are handled properly.
Exceptions have no real advantage over error-code model. Most people use it because they have to, or because they believe it’s an "advanced" feature.
Program for *transactions*… and not necessarily only database transactions, but program with the whole transaction concept in mind. Either an entire operation must complete successfully or the whole thing fails. Always ask what happens if only a part of the transaction fails. Program not only to correctly handle internal exceptions, but consider how your component might fit into a larger transaction. Is it possible to rollback what your code has just done if later code requests that it be cancelled? Think transactions…
"Right, my point is that it isn’t any easier. It’s different. And in my opinion, harder since the error handling is much more subtle."
I personally find it a lot easier to do a line-by line "can this throw a non-hardware error (out of memory, etc.)" inspection than decoding what’s effectively a giant if/then error handling jump table.
I don’t see in this case how using error status checking would resolve the half-created object problem, either, unfortunately.
"I personally find it a lot easier to do a line-by line "can this throw a non-hardware error (out of memory, etc.)" inspection than decoding what’s effectively a giant if/then error handling jump table."
True. However if you’re coming to code that’s already been written, you have to do this analysis all over again since the absence of recovery code could mean "Somebody thought about this and decided no recovery code was necessary" or it could mean "Nobody thought about this." Few people add the comment like "it’s okay if this throws an exception because xyz" to their code.
Error code paradigm is much wordier – checking error codes everywhere – but it’s obvious when somebody forgot to check an error code. And in the cases where somebody wants to ignore an error code, they usually add a little comment "ignoring error code because xyz".
Jason McCullough,
Another easier part is that with error codes you know exactly where the error happened and should know how to recover from that point.
With exceptions, you have to do a lot of detective work to figure out where you were.
Part of the commit/rollback problem is why I like Fowler’s Unit of Work pattern, though I have yet to use it.
It’s funny. This exact scenario happened today when I was debugging some framework code from our collaboration partner. The main form load method was parsing an XML file which was malformed, and the uncaught exception caused it to skip asking the user to log in (as well as not closing an annoying splash screen.)
I’ll definitely agree with that and add that there are never any *magic* fixes in writing code, be it using exceptions or something else. Exceptions aren’t a panacea, but they do help the programmer add structure to exception handling. I guess where I thought this was going was that Raymond was going to offer us a better pattern. The one thing I like about the solution I postulated was that you let the setters take care of the bidirectional association and rollback in case of failure. I like this approach better than having one monolithic method that sets Guy’s Team and Guy’s League. That way the client call isn’t burdened with state management for a bunch of objects, but rather can simply react to a failure in whatever way makes sense for the app and be confident that the system is in a consistent state. I, for one, think this approach is broadly applicable and hence was confused when Raymond pointed out that it wasn’t generalizable.
this seems short sighted.
as a wise person i once knew said:
its as wide as it is long.
"Right, my point is that it isn’t any easier. It’s different. And in my opinion, harder since the error handling is much more subtle."
you can make the same argument for fortran over oo languages, observe:
s/it/oo languages/
s/error handling/flow control/
.
.
.
i bet you can fill in the rest of the details.
also,
dear raymond,
i am sorry for not consolidating my comments into a single post. i am a thoughtless individual. spurn me.
-nathan
Maybe this isn’t a good (too simple?) example — but if you handled this in a DBMS the transaction logic would handle every error case.
MR, Raymond’s just using this case as an example. It’s a strawman to illustrate a point.
And doing this in a dbms would be a LOT more complicated.
The discussion here isn’t about how to solve a problem. It’s about the fact that exception handling doesn’t make issues with correctness go away.
Please take careful note that this is now at least four senior Microsoft bloggers that’ve commented that "Exception handling isn’t a panacea" either on this thread or on other threads (Raymond, Eric Lippert, Michael Grier (http://weblogs.asp.net/mgrier/archive/2004/02/18/75324.aspx) and now me (assuming I can play in the same league as the other three)) And other Microsoft people who aren’t bloggers have also made similar comments.
Trust us :) Exceptions don’t make a developers job any easier. They’re a different paradigm than return code checking, but…
Hmm… I think that, even if one reaches the conclusion that "all unexpected exceptions are fundamentally unrecoverable, and the only safe thing to do is terminate the process", then still, you gotta admit that exception-handling makes *that* chore a bit easier. :)
For the most part, that’s all I use it for — I establish one big catch-all near the top of a thread’s stack frame (which does nothing but attempt to log the error or display a msgbox or whatever, and then exit).
Then, I take care to wrap calls to any especially exception-prone functions (such as file- and network-i/o) in a more strongly-typed catch-block, closer to the bottom of the stack frame…
During testing and debugging, if an unexpected exception occurs, I investigate it as I would any other type of "crash" bug, and take steps to guard against it — maybe with a try/catch block, or maybe by doing better validation, etc.
That basic strategy gets me reasonably solid code, for a reasonably small amount of typing — I’m satisfied with it.
I do, however, regret that there are so many "especially exception-prone functions" in the FCL which don’t return explicit error-information. Many features of the FCL force us to use exception-handling as flow control, and that’s wrong. A try/catch block is almost always more cumbersome than an if statement.
But they’re both equally easy to forget, or neglect.
nathan: I guess the difference between exceptions and object-orientation is that introducing object-orientation into your coding is perhaps more ‘fail safe’ than introducing exception handling.
With exceptions, you pretty much have to decide that you’re going to handle exceptions properly everywhere, otherwise subtle cases fall through the cracks. So it’s difficult to merge with non-exception codebases, and to introduce yourself to as a developer slowly. In addition, as Raymond has illustrated, exception handling has some pretty subtle failure modes, while at the same time giving you a false sense of correctness.
With object-orientation you can start to use pieces of object-oriented methodology a piece at a time, without having to convert all the code it touches. So you can introduce yourself to the concepts slowly. And it tends to be more ‘fail safe’, the failure modes are more immediately obvious, and the subtle ones are less catastrophic (e.g. poor performance rather than incorrect behavior).
At core, object-orientation is about reducing the amount of global knowledge you need to have. You only deal with an object’s interface, without having to worry about its implemention, or whether it might be polymorphic. That’s clearly a helpful paradigm.
Proper implementation of exceptions involves just as much non-local knowledge as traditional error handling, it’s just dressed up to give the illusion otherwise. Which is, in my opinion, why so many problems arise.
I’m not familiar enough with C# to comment, but Java suffers from a very similar problem. That is, *no operation can be guaranteed not to throw*. This makes it so hard to write classes that offer any of Abraham’s exception levels. It even makes it hard to maintain consistency in any non-trivial object. And I should note that this isn’t even directly related to Exceptions vs Error Codes per se, it has more to do with the fact that these failures can occur at any point.
In Scott Meyers’ excellent "Effective C++ CD", he includes some magazine articles. Here are his introductory comments on a 1994 article on exceptions:
""Exception Handling: A False Sense of Security" by Tom Cargill. It’s hard to overestimate the influence this article had on the C++ community when it appeared in the November-December 1994 issue of the °C++ Report. At that time, most C++ programmers thought there was little more to exceptions than try, throw, and catch. Tom showed just how wrong that was. (They didn’t call his column "C++ Gadfly" for nothing.) He demonstrated that writing exception-safe code isn’t just hard, it’s really hard. In addition, Tom challenged the C++ community to come up with rules for writing exception-safe programs. It took years before those rules began to appear in print, and the difficulties Tom identified affected work on the standard C++ library until the final days of its specification."
Here is the link for the CD:
http://www.awprofessional.com/titles/0-201-31015-5/
Låter det konstigt, men jag är faktiskt intresserad av Exceptions? Alltså felhanteringen i .NET. Men vad är egentligen exceptions och när och hur skall dessa användas? Vid första anblicken kanske detta inte verkar vara någon svår fråga att svara på,
*cough*AOP*cough*
The point should also be made that, from a performance standpoint, exceptions can be expensive (at least they can be in Java; not sure how difference C# is on this). It’s much more efficient to check for erroneous conditions in logic than to wait for something to break. For example, if you’re accepting user input that’ll be stored as a numeric type, make sure (via code) it looks like a number before trying to cast it as one. The compiler may require you to use a try/catch around the cast, but you can reduce the likelihood that you’ll actually get an exceptional condition.
Perhaps there’s more going on here than just error codes vs. exceptions. I suspect this has more to do with a procedural vs. OO paradigm delta than the specifics of exceptions.
1) Your first example up there, the create database one from the book, is a procedural definition of constructing something; why not just use a constructor? Failed constructors have the nice benefit of automatic unwinding; cleaning up should be the responsibility of the object, not the person creating a copy of the object, unless you’re dealing with a very strange boundary case. Not sure why it was written that way, unless it was a static object factory method. Which begs the question of why they didn’t write cleanup code for their library.
2) "The system is too screwed up to handle the error" scenario has no sigificant differences in the error handling or exception paradigms; if the file system just blew up, you can’t clean up files while handling a file system exception or a file system error.
3) Of course trivial exception handling won’t work if you partially commit objects, such as your AddNewGuy() example.
"With exceptions, you have to do a lot of detective work to figure out where you were."
If you have to do detective work to figure out where you where, You’re Doing It Wrong. ™
*cough* RAII *cough*
Jason: "Failed constructors have the nice benefit of automatic unwinding". Um. Forgive me, but if you do:
class Foo
{
Foo()
{
CreateDatabase();
CreateIndexes();
CreateTables()
}
}
Where is the automatic unwinding done if CreateTables throws? How is the database created in CreateDatabase() deleted?
Moving work into a constructor doesn’t fix the problem.
If you write a destructor it does fix the problem and that’s RAII.
God, it’s disturbing how many programmers never heard of it. "final" or "finally" is such a lousy concept compared to it, but obviously it’s above certain people.
The problem with using the destructor is that the destructor is nondeterministic in the CLR.
From the RAII web page, ".NET uses non-deterministic garbage collection, much like Java, and RAII techniques are not directly applicable to .NET."
Andrei Alexandrescu and Petru Marginean have a good C++ solution to this problem:
http://www.moderncppdesign.com/publications/cuj-12-2000.html
In the context of this problem it’d look something like this:
Guy guy = new Guy(name);
guy.Team = ChooseRandomTeam();
guy.Team.Add(guy);
ScopeGuard guard = MakeObjGuard(guy.Team, &Team::pop_back);
AddToLeague(guy);
guard.Dismiss();
return guy;
You still have to think about what’s happening, but the solution isn’t necessarily ugly.
It just seems to me that there are all these contortions suggested to make exceptions work correctly, that it’s somewhat defeating the purpose.
Exceptions were supposed to make error handling easier and less, well, error prone. In practice, it’s not entirely clear to me that they succeed.
The question is not whether error handling can be done correctly with exceptions. Of course it can. The question is, once you did that, was it really any easier than just straightforward procedural error-code propagation?
I have got to say that I disagree wholeheartedly with the "error code" approach. Using an error code because it let’s you know exactly what went wrong completely ignores custom exceptions. The type of the exception along with all the rich information that accompanies it can tell you much more than a code. Exceptions *can* also make code easier to understand by allowing one to structure the program in a way that clearly separates normal logic from error conditions. The point is poor error handling can be written using either paradigm, but I believe the judicious use of excpetions introduces benefits that outweigh the risks.
*cough* an acronym is not an argument *cough*
class Foo
{
public:
Foo () : database (), indexes (), tables () {}
private:
Database database;
Indexes indexes;
Tables tables;
}
So how does the Database know whether it’s supposed to delete the file during destruction or whether it’s supposed to let the file continue to exist?
Raymond, your post is a pretty strong argument for checked exceptions. If exceptions are not checked, you need to rely on the programmer to check the documentation and hope that the said documentation is up to date…
Guy AddNewGuy(string name)
{
Guy guy = new Guy(name);
guy.Team = ChooseRandomTeam();
try
{
guy.Team.Add(guy);
try
{
AddToLeague(guy);
}
catch (Exception ex) // should be explicit
{
if (LeagueContains(guy))
RemoveFromLeague(guy);
throw;
}
}
catch (Exception ex) // should be explicit
{
if (guy.Team.Contains(guy))
guy.Team.Remove(guy);
throw;
}
return guy;
}
Like most of the "Exceptions are Bad" posts I’ve seen, the problem isn’t that exceptions are bad: the problem is that your code is bad. You don’t carry the state with you through each method. It should be written like this:
<pre>
public void GenerateDatabase()
{
Database d = lookupDatabase();
try {
CreatePhysicalDatabase(d);
CreateTables(d);
CreateIndexes(d);
}
catch ( Exception e ) {
d.undo();
}
finally {
d.release();
}
}
</pre>
Tim: Hooray. Now if only people actually wrote code that way. Certainly the most popular books don’t.
(Though you forgot the rethrow, and you forgot to wrap the undo() call in its own exception handler.)
_: You get deterministic destruction but also the burden of invoking it at the right time (and the correct number of times):
Guy& GetLoaner()
{
Guy g(…);
return g; // oops
}
It’s more complicated if GetLoaner() sometimes returns an existing guy and sometimes returns a new guy. How is the caller to know whether it should be destructed when the caller is finished with him? You’ve traded one complexity for another.
I am inheriting a codebase where their idea of exception handling is to:
bool l_bReturn;
try
{
//do work
l_bReturn = true;
}
catch (Exception &e)
{
//log message
l_bReturn = false;
}
return l_bReturn;
Not only are we catching ALL errors (including access violation which IMHO means the user data can no longer be assumed to be 100% correct), we are ignoring the actual error and just logging the message. It makes for nice stack traces but one of the big points of exception handling is to not have to check return values.
So not only are we ignoring errors and probably leaving "operations" partially done, we are still depending on people checking return values.
I really need a good paper or book that I can give to them to help me win my case that this is a very bad thing. Well, IMHO. :)
"Raymond, your post is a pretty strong argument for checked exceptions."
No it isn’t. There are AFAIK no good arguments in favour of checked exceptions.
Checked exceptions don’t solve this issue at all. Checked exceptions enforce catching of exceptions (or writing code to say /yes, I really did want this to escape/). They don’t enforce correct handling of exceptions (catch(Exception e) {} is rife in Java), and they don’t enforce transactional semantics.
<b>
Cleaner, more elegant, and wrong.
Suppose an exception is thrown during CreateIndexes(). The GenerateDatabase() function doesn’t catch it, so the error is thrown back out to the caller, where it is caught.
But when the exception left GenerateDatabase(), important information was lost: The state of the database creation. The code that catches the exception doesn’t know which step in database creation failed. Does it need to delete the indexes? Does it need to delete the tables? Does it need to delete the physical database? It doesn’t know.
So if there is a problem creating CreateIndexes(), you leak a physical database file and a table forever. (Since these are presumably files on disk, they hang around indefinitely.)
==================================
</b>
[How I can "quote" the message I am replying to?]
Yo can’t say for sure that the solution referred is wrong. It would be quite right if you catch the exceptions in each of CreatePhysicalDatabase(), CreateTables(), CreateIndexes() routines, run the local error-handling code, and then propagate only the exceptions you need to the outer level (to the caller). Actually, that in most cases should mean not just re-throwing the exception catched, but throwing another one.
"Checked exceptions don’t solve this issue at all. Checked exceptions enforce catching of exceptions (or writing code to say /yes, I really did want this to escape/). They don’t enforce correct handling of exceptions (catch(Exception e) {} is rife in Java), and they don’t enforce transactional semantics."
No solution can be presented if programmers bend the rules in the wrong direction on purpose. Your "argument" against checked exceptions is the same as catching an error code and then do nothing with it.
The original sample suffers from the same problem. The exception mechanism is used in an obvious false manner. The same sample could be produced by not catching error-code return values (or not doing anything with them), then saying "error codes are flawed bla bla bla".
One argument against exceptions stated is that there is less state available to the error handling party. The beauty of exceptions is a matter of fact that it is an object which can be filled with all kinds of specific error information, provided you define a subclass for it. Using return codes, which is usually an int or a bool, actually provides much much less information, and in a much less defined way as well (error const hell anyone?).
Well LarryO already cited my blog but the thing here is that even most C and C++ developers don’t understand "how to use exceptions correctly". Java and C# don’t even have the basic language constructs necessary to make them work. (See the quoted blog entry for details.)
Moving towards a more database/transactional-workspace kind of design is interesting; I built a system like that back at Digital but the per-frame overhead of managing transaction state is excessive for general purpose programming.
The thing is this. This code is obviously probably buggy:
HRESULT hr = foo();
if (SUCCEEDED(hr)) hr = bar();
if (hr == WHATEVER_E_NOTFOUND) {
// aha, foo didn’t find it!
…
}
it’s not obvious if this code is buggy
try {
foo();
bar();
} catch (NotFoundException e) {
// aha, not found!
…
}
it’s totally unobvious that this code is buggy:
try {
foo(a, b, c);
} catch (NotFoundException e) {
// aha, foo didn’t find it!
…
}
(The last one is buggy because implicit conversions may have occurred coercing the parameters for the function call.)
I think that something that would help people see this much more easily would be to be able to overlay a (potential) control flow diagram on top of the source code. Seeing all four independent control flow arrows from the foo(a,b,c) line to the catch statement would help highlight the problem.
Now, of course, the real problem is that changes to the implementations of the types of a, b and c may arbitrarily rearrange the control flow. You might want to try to write:
RealFirstParameterType p1 = a;
RealSecondParameterType p2 = b;
RealThirdParameterType p3 = c;
try {
foo(p1, p2, p3);
} catch (NotFoundException e) {
// whew, only foo could have thrown this!
…
}
but that comment is a lie. You have no reason to believe that the same overload that you used to call is still the one selected. Also if you believe destructors can throw in C++, foo could have returned an object which was destroyed and originated the NotFoundException.
I used to think that exceptions were the best thing since sliced bread. I now conclude that it’s almost impossible to write high quality code that uses them. Yes, it’s possible, I’ve done it and one of my past teams where we sold exceptions heavily have done what is necessary to do it. But man, it’s not easy and it’s really not easy for people to walk up to the code and grok it.
Any language feature won’t work well in some situations. Even in C++, every time you call operator new, you have to think about exceptions. The problem with approaching exceptions from the perspective of C# and Java is that you don’t have proper destructors, so you’ll never see a place in which exceptions really shine. And even then you have to trust yourself and your teammates to understand them (just like any other language feature).
As an aside, this notion that "the GC will clean it up" is downright quaint. Memory is the least interesting resource I manage. Why are people so obsessed with it? I suspect it’s merely easy to talk about.
"Moving towards a more database/transactional-workspace kind of design is interesting."
Using the "using" statement of C# this is quite easy to do already in C#:
using (Trans trans = new Trans()) {
trans.doStuff(a + b);
trans.commit();
}
This example is actually a form of exception handling as well. The trans will always be disposed immediately after the using statement, and it’s to check in the trans if the commit was called and executed properly. Difficult to introduce bugs using this approach.
The most bugs I see in everyday work are objects which don’t manage their state properly. Improper exception use may be one of the symptons, but they are not the sole cause of objects going into an invalid state (without throwing exceptions BEFORE that happens). It’s just a matter of knowing what the hell you are doing…
Using does not suffice. The CLR (not C#) lacks the facilties to do exception-free rollback, either via IDisposable or try/finally.
In C++ it’s possible but it’s ugly and people don’t know how to read/write/maintain the code.
It could be that we’ve lived with statuses for so long that it’s the devil we know vs. the devil we don’t know but it’s much easier to spot the bugs in status based source code than exception based source code.
How bout this…
Assuming class Team has an ID that uniquely identifies it in an array…
Guy AddNewGuy(string name)
{
Guy guy = new Guy(name);
Team t = ChooseRandomTeam();
t = t.Clone(); // doesn’t need to be deep…
t.Add(guy);
AddToLeague(guy);
// ID is just an index into an array
UpdateTeam(t);
return guy;
}
You clone the Team which doesn’t need to be deep because you don’t care about copies of the guys, just the underlying array. Add the guy to the clone, then add the guy to the league. If the league fails, the team that is stored in the array (behind the scenes of ChooseRandomTeam) isn’t modified. The call to UpdateTeam(t) simply updates the team in the array with the new team. The old team object gets GC’ed later on. This even protects against Array.SetValue throwing an exception as long as any exceptions happen before the underlying memory is touched.
Now you are cloning the team object which could cause a large memory allocation.
Thoughts?
4/27/2004 10:59 AM Pete Gontier:
> As an aside, this notion that "the GC will
> clean it up" is downright quaint. Memory is
> the least interesting resource I manage.
This reminds me of Dr. Pizza’s statement in another thread. Finally I understood the intent of his/her statement, and yours looks similar. But memory management still cannot be ignored.
With some kinds of handles you can call DuplicateHandle(). When each user no longer needs the object they can call CloseHandle(). When the object’s last handle gets closed the object might be suitably disposed of. For kinds of handles which cannot be passed to DuplicateHandle(), and for Schroedinger’s handles (kinds of handles where MSDN doesn’t say if you can pass them to DuplicateHandle()), the situation is tougher.
With memory management, when one DLL needs to pass a pointer to another DLL and neither one really knows when the object can be disposed of, you get blue screens and/or leaks. GC has a purpose.
On the other hand, in an embedded system that I coded in C++, destructors take around 16% of the CPU time and constructors take around 14%. The only solution I can find at present is to re-code the whole thing in C, allocate each temporary object once (or make them static variables if sizes are known at coding time), and consider freeing them once if there’s going to be a shutdown operation other than power-off. Of course a 25% reduction in CPU time isn’t going to be enough, but it’s the first thing to do because it’s the biggest waste. Yes memory is the least interesting resource, but memory doesn’t care whether I’m interested or not, I have to deal with it.
<rant>
I think this is more than Exception Handling.
The sample code itself stinks whether error handling is involved or not. When I saw the sample code, my nose perked up and detected foul odor.
> CreatePhysicalDatabase();
> CreateTables();
> CreateIndexes();
I would like to call this ‘blind calling’ or ‘implicit calling’ because you don’t know how the system is changing or evolving. There is no return value or no parameter. This anti-pattern usually shows up the procedural language but it also frequently shows up in OO language.
‘Blind calling’ anti-pattern is bad because you don’t know what aspect of the system the caller is affecting. In OO, an object is affected by external caller’s message. So the caller knows that he/she is changing or affecting the object. But the original example is basically old procedural function calls masquerading to be OO. The object is calling private methods blindly not knowing affected actor/part of the system.
You have to treat these private methods as the same as object method. Pass in the object that you want to have work done. Unless you do that, you are just assuming that the function is doing something for you.
Basically, ‘blind caller’ anti-pattern has the same behavior as old global variable anti-pattern – you don’t know what is going on.
That’s why Tim Burns’ solution is excellent.
http://weblogs.asp.net/oldnewthing/archive/2004/04/22/118161.aspx#119473
This shows that each function call will affect the database server. You are delegating method to work on the database.
This is a bad programming exercise.
</rant>
This goes to the basic assumption – if you write a good program with a few smells, it is easier to implement correct ‘Exception’ handling.
I guess it is a universal truth of programming – it does not matter how well language is designed – it is still people who is writing it. Garbage In – Garbage Out.
My team has had great success using nested blocks with explicit error handling required. This is a methodology that is compatible with both exceptions and error codes (6 of 1, half dozen of the other). We use error codes because we’re writing C++ COM objects so it’s easier to be consistent across the board. Our code for this problem would look like:
HRESULT hr;
AccessDatabase *accessDb = new AccessDatabas();
if( accessDb != NULL )
{
hr = accessDb->GenerateDatabase();
if( FAILED( hr ) )
{
delete accessDb;
}
}
else
{
hr = E_OUTOFMEMORY;
}
HRESULT accessDb::GenerateDatabase()
{
HRESULT hr;
hr = CreatePhysicalDatabase();
if( SUCCEEDED( hr ) )
{
hr = CreateTables();
if( SUCCEEDED( hr ) )
{
hr = CreateIndexes();
if( FAILED( hr ) )
{
DestroyTables();
}
}
if( FAILED( hr ) )
{
DestroyPhysicalDatabase();
}
}
}
If we were using exceptions, it would look like this:
AccessDatabase *accessDb = new AccessDatabas();
try
{
accessDb->GenerateDatabase();
}
catch( e )
{
delete accessDb;
throw e;
}
void accessDb::GenerateDatabase()
{
CreatePhysicalDatabase();
try
{
CreateTables();
try
{
CreateIndexes();
}
catch( e )
{
try
{
DestroyTables();
}
throw e;
}
}
catch( e )
{
try
{
DestroyPhysicialDatabase();
}
throw e;
}
}
At the end of the day, we think the error code version is easier to read and understand (perhaps because we’re more familiar with it). But either way, the thinking that needs to go into the problem is the same, either way.
– Geoff
Raymond, you make me wonder why the exceptions aren’t a problem in Standard ML, the language Bjarne Stroustrup got them from.
I really am puzzled.
Could it be because ML programmers are brighter than Mort and hence better programmers?
Could it be that ML is much more coherent language and nice and functional and stuff?
Don’t seem like good answers to me (even though they typically /are/ much better programmers and ML really /is/ a much more coherent language than the chimera C++).
Thank you for planting this thought – I’ll probably think about it on/off for the next month or two :)
Functional languages are where most of the exception "research" if you will call it that comes from and note something very very interesting – the complexity of error path rollback is entirely due to side effects. memory allocation, starting a database transaction, changing fields in a structure in memory, all side effects.
When you lack side effects, transactional rollback is trivial. When all your side effects are managed by something like a database whose sole job in life is, if nothing else, to preserve the ACID transactional characteristics, "all" you have to do is remember to abort the transaction. (Which is someething that the "using" syntax paradigm is absolutely perfect for.)
My personal credo: " Just because you missed an error path doesn’t mean you can pinpoint its source from a user support call as easily as an exception that was only caughed by your global or top-level catch-all exception handler. "
As for the example: there is only one side effect that needs to be handled in an lower-level handler, and that is the Access Database (the MDB file) being left in an inconsistent state.
So instead of cluttering up the GenerateDatabase method with all sorts or error code return value checks, all that’s needed is to cleanup in case of an exception. No matter WHAT happens, you will always be able to pointpoint the cause, line number, etc, provided you have at least a global/ thread handler.
string mdbFile = @"c:mydb.mdb";
AccessDatabase accessDb = new AccessDatabase(mdbFile);
using(accessDb) {
accessDb.GenerateDatabase();
}
}
public void GenerateDatabase()
{
try {
CreatePhysicalDatabase();
CreateTables();
CreateIndexes();
}
catch (Exception e) {
if (File.Exists(this.mdbFileName))
File.Delete(this.mdbFileName);
throw new Exception("Could not generate: " + this.mdbFileName, e);
}
}
This is EXACTLY the problem that scope guard solves. You are using the wrong language. This can be solved in C++.
Commenting on this article has been closed.
It may be easier for you, but it makes it harder for everyone else.
It’s not that exceptions are inherent bad, it’s that it’s hard to code to them and hard to tell that somebody did it right.
Readify does apparently. We have a technical mailing list internally that most of the technical folk
PingBack from http://soci.hu/blog/index.php/2007/04/18/apro-korultekintes-a-vista-forraskodban/
PingBack from http://mikesdump.wordpress.com/2005/05/11/error-handling/
PingBack from http://www.brainfarters.com/?p=54