October 17, 2003
@ 04:30 AM

After Joel Spolsky's recent post on exceptions which generated lots of dissenting opinions he has attempted to recant in a way that leads one to believe he is bad at accepting critical feedback.

In his most recent posting Joel writes

And Back To Exceptions

There's no perfect way to write code to handle errors. Arguments about whether exception handling is "good" or "bad" quickly devolve into disjointed pros and cons which never balance each other out, the hallmark of a religious debate. There are lots of good reasons to use exceptions, and lots of good reasons not to. All design is about tradeoffs. There is no perfect design and there is certainly never any perfect code.

His response is a cop out and not something I'd expect from a professional software developer who had been shown the errors in his proposed solution. This non-recant actually lowers my opinion of Joel in ways his original post did not. At least the original post could be based on past experience with bad uses of exceptions or usage of exceptions in programming languages where they were bolted on such as C++. His newest post just sounds like he is trying to save face instead of critically analyzing the dissenting opinions and showing the pros and cons of his approach and that of others. The best part is that he picks perhaps the worst defense of using exceptions to counterpoint against

With status returns:

STATUS DoSomething(int a, int b)
{
    STATUS st;
    st = DoThing1(a);
    if (st != SGOOD) return st;
    st = DoThing2(b);
    if (st != SGOOD) return st;
    return SGOOD;
}

And then with exceptions:

void DoSomething(int a, int b)
{
    DoThing1(a);
    DoThing2(b);
}

Ned, for the sake of argument, could you do me a huge favor, let's use a real example. Change the name of DoSomething() to InstallSoftware(), rename DoThing1() to CopyFiles() and DoThing2() to MakeRegistryEntries().

Since I'm always one to take a challenge here goes how I'd probably write the code

void DoSomething(string name, string location)
{

try{
    InstallSoftware(name);
    CopyFiles(location);
    MakeRegistryEntries(name);

  }catch(SoftwareInstallationException e){

  RollbackInstall(name);

  }catch(FileIOException fioe){

  RollbackFileCopy(location); 

  RollbackInstall(name);

 }catch(RegistryException re){

   RollbackRegistryWrite(name);

   RollbackFileCopy(location); 

   RollbackInstall(name);

  }

DisplayInstallationCompleteMessage(name, location);

}

or even better

void DoSomething(string name, string location)
{

    try{
    InstallSoftware(name);
    CopyFiles(location);
    MakeRegistryEntries(name);

   }catch(Exception e){

     throw InstallationException(e, name, location);

  }

DisplayInstallationCompleteMessage(name, location);

}

Of course, you could ask what happens if a rollback attempt fails in which case it either should catch the exceptions in itself or throw an exception. Either way, I prefer that the above approaches to littering the code with if...else blocks.


 

Friday, 17 October 2003 06:19:08 (GMT Daylight Time, UTC+01:00)
Here's how do do it with goto and status returns:

STATUS DoSomething(string name, string location)
{
STATUS st = S_OK;

    st = InstallSoftware(name);
if (st) goto ErrorInstall;

    st = CopyFiles(location);
if (st) goto ErrorCopyFiles;

    st = MakeRegistryEntries(name);
if (st) goto ErrorRegistryEntries;

DisplayInstallationCompleteMessage(name, location);
return st;

ErrorRegistryEntries:
   RollbackRegistryWrite(name);

ErrorCopyFiles:
   RollbackFileCopy(location);

ErrorInstallSoftware:
   RollbackInstall(name);

return st;
}

There are no if/else blocks. Cleanup code is written once and not duplicated in each except clause. Error handling is collected in one place at the bottom of the function.
Friday, 17 October 2003 10:42:41 (GMT Daylight Time, UTC+01:00)
Am I missing something here?

Shouldn't InstallSoftware(), CopyFiles() and MakeRegistryEntries() be responsible for rolling back any side effects in case of an exception. Why must the caller take care of that?
Lars Jonsson
Friday, 17 October 2003 13:07:09 (GMT Daylight Time, UTC+01:00)
Lars,
You're right, I need to be smacked across the face with a copy of "Exceptional C++". In proper Exception-safe code each object should put itself in a consistent state before throwing an exception. However in practice when using libraries from others this isn't always the case and the caller still needs to do some cleanup.

Gary,
The goto solution works just as well except that with exceptions I have the benefit of being able to log stack information for debugging purposes while with gotos I don't.
Friday, 17 October 2003 23:54:27 (GMT Daylight Time, UTC+01:00)
re: Consistent State
InstallSoftware(), CopyFiles() and MakeRegistryEntries() are functions in the example, not objects. It is good design practice for each function to leave the system in a consistent state with respect to the changes made by the function. In order for the DoSomething function to leave the system in a consistent state, it might need to rollback the work done by InstallSoftware and CopyFiles. You need to write code for this. There's no automagic way to make this happen.

Assuming that each function leaves the system in a consistent state, then there's no need for the call to RollbackRegistryWrite.

re: Logging stack trace
The benefit of exceptions is that there is an out of the box solution for stopping in the debugger when an error condition is detected. It is very easy to craft macros to accomplish the same thing with the if / goto / status solution.

As far as logging stack traces is concerned, exceptions do not do this out of the box. At least the systems I have used do not do this out of the box. Extra code is required for logging with or without exceptions.

Saturday, 18 October 2003 16:26:15 (GMT Daylight Time, UTC+01:00)
Gary,
In the systems I have used (C# and Java), the base Exception class comes with a the ability to print out the stack trace for where the error occured complete with line number information. This has proved useful when users of RSS Bandit report bugs since they can also send an error log with information about where exactly the error occured which is a lot better than having just a description from the user.
Sunday, 19 October 2003 00:16:41 (GMT Daylight Time, UTC+01:00)
It is possible to grab a print a stack trace from a plain old C program. It's not easy to figure out how do do this on the Windows platform, but it's possible. It is certainly easier to get a stack trace using the C# and Java Exception base classes.

Is logging the same thing as printing, or is logging something more than printing? If logging is something more than printing, do exceptions provide the additional functionality or is the additional functionality independent of the exception mechanism?
Sunday, 19 October 2003 01:23:54 (GMT Daylight Time, UTC+01:00)
Gary,
For my use cases logging is just printing.
Monday, 20 October 2003 23:28:00 (GMT Daylight Time, UTC+01:00)
Really if you code so as to cleanup within each method, ie you *can* get back to a known good state from a method, then you don't need to thrown an exception, and you definitely don't want to be using them for short-circuit control flow. Exceptions are for control flow you genuinely surprised about, but have considered possible anyway.

Methods on objects that are to be composed as part of a 'protocol' should consider returning success or failure assuming you've thought out the protocol (which can be tedious and/or time consuming).

void Install(string name, string location) {
try {
SystemSnapshot(name, location);

if(InstallSoftware(name))
if(CopyFiles(location))
MakeRegistryEntries(name);

DisplayInstallationStatusMessage(
name, location getInstallationStatusMessage());

} catch(InstallationCatastrophe e) {
BestEffortRollback(name, location);
DisplayInstallationCatastropheMessage(
name, location ""+e);
}
}

While it's more wordy than a flat sequence of calls wrapped in single exception block, I prefer the above because it makes the installation protocol explicit in the code - it's clear that,

o you shouldn't enter CopyFiles() if InstallSoftware() does not succeed.

o you should report back sucess or failure post-installation

o if something really bad happens, you should make an effort to clean up before reporting back about that really bad.


[As an aside I've noticed that people who are good with exceptional code tend to have a good head for comms protocols and networking.]

Comments are closed.