views:

218

answers:

6

I'm kind of torn between these two error-handling models:

  1. Create a boolean Error and a string ErrorMessage property for your object. Catch all exceptions internally in the object's methods and pass the messages along using conditional logic from the caller, ie:

    Dim o As New MyObject
    o.SomeMethod()
    If Not o.Error Then
        'Do stuff'
    Else
        Dim msg As String = o.ErrorMessage
        'do something with message'
    End If
    
  2. Throw exceptions in your object and handle them on the outside with Try Catch logic:

    Dim o As New MyObject
    Try
       o.SomeMethod()      
       'Do stuff'
    Catch ex As Exception
        Dim msg As String = ex.ErrorMessage
        'do something with message'
    End Try
    

To me, it seems like the same amount of code either way, except that you have property code for the Error and ErrorMessage properties. However, you also can tell when an error occurs without having to check for exceptions. Which pattern should I go with?

+2  A: 

The biggest issue I have with the first one is that it's passive, easily overlooked and not very standardized. How will a programmer know to check that property? Or which properties / methods can possible set an error? Or which property / method access caused the error to be set?

For example. In your first sample code if o.Error is True, it's unclear whether the initialization of the object or the call to SomeMethod caused the flag to be set.

The exception model is an unignorable way of telling your users that an error occurred. It cannot be avoided without explicit code to handle the situation.

JaredPar
i've started to implement the second model and have realized that a lot of internal error handling can be removed, since the exception can be handled on the outside
Jason
I think history is informative on this. The Win32 API uses techniques that are similar to the first method, like `GetLastError`, only it's a nightmare because it's not very standardized. That's part of the reason why Microsoft invented something called .NET and decided on structured exception handling throughout the runtime, no exceptions! :)
MarkJ
+7  A: 

I have decided to go with throwing exceptions instead of using error/return codes. I just recently looked really hard into this.

The #1 reason to throw exceptions is there is a possibility you can forget to check the error code. If you don't check it, then you will continue working while the error exists. With exceptions though, if you forget to handle them, then the exception will raise to the top and stop all processing. It is better for this to happen than to continue after unknown errors have occurred.

For more info check out the Exception chapter in Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries, Second Edition by Addison-Wesley.

Joel Spolsky actually prefers error/return codes over exceptions but a lot of people disagree with him. Joel's post in favor of return codes can be found here. Check out this blog post and all of the comments with some good discussion regarding this subject.

Dennis
+1: Great answer!
TrueWill
+3  A: 

Prefer #2. For details, see this excerpt on Exception Throwing from the development of Microsoft's excellent Framework Design Guidelines, as Dennis mentioned. Note especially the section on Exceptions and Performance.

Short version:

Do not return error codes.

Do report execution failures by throwing exceptions.

Do not use exceptions for normal flow of control.

I highly recommend reading the book for a full discussion, complete with commentary from a number of the Microsoft luminaries.

TrueWill
FYI, the book is available on Safari Books and is an excellent read on many different subject matters.
Dennis
If you pick up the Framework Design Guidelines (and I highly recommend it), make sure to get the second edition.
Scott Dorman
@Scott: Absolutely.
TrueWill
+3  A: 

Exceptions should be used when something exceptional has happened.

e.g. you are passed a null (nothing) object when you expect one.

Uncle Bob recommends Exceptions over Error codes in his book Clean code.

He says

The problem with these [error codes] approaches is that they clutter the caller. The caller must check for errors immediately after the call. Unfortunately it's easy to forget. For this reason it is better to throw an exception when you encounter an error. The calling code is cleaner. Its logic is not obscured by error handling.

John Nolan
but don't you still have to run a `Try Catch` block anyways? this is the same as running an `If/Else` statement, right?
Jason
Exceptions aren't for communicating **exceptional** conditions they are for communicating **error** conditions. This might be a subtle difference, but it's an important one. What is exceptional for one application might not be exceptional for another. Handling null reference exceptions actually should be avoided as these almost always indicate a bug in the code and should be fixed in the code so they don't occur.
Scott Dorman
+1 for mentioning Clean Code - that's a (heh) SOLID read.
TrueWill
@Jason: What usually ends up happening is that rather than try/catch blocks you use more try/finally blocks. Don't use catch handlers for cleanup as they aren't guaranteed to run. Finally blocks will run no matter what so they should contain your cleanup code. You only want to handle an exception at a point when you can actually do something about it; otherwise you should let it bubble up the call stack. If you must log the exception at some lower point, be sure to rethrow it using just the `throw` keyword (not `throw ex`) to preserve the stack trace information.
Scott Dorman
+2  A: 

They are both accepted forms of error handling, however the preferred choice for .NET languages is to use exceptions.

There are a few problems with using return codes (either numeric or boolean), the two biggest being:

  • Easily overlooked/ignored by programmers.
  • Can't be used in all situations. What happens if your constructor fails? It's not possible for you to return a value explicitly from a constructor.

For these reasons alone, you should use exceptions. Exceptions provide a clean, standardized way to indicate and any failure no matter where it arises.

You will also end up with less code overall as you should only catch exceptions when and where you can safely and appropriately handle that exception.

Scott Dorman
+1  A: 

I recommend using both.

Why?

"Use the right tool for the job"

The "problem" with return codes is that people often forget to handle them. However, exceptions don't solve this problem! People still don't handle exceptions (they don't realise a certain exception needs to be handled, they assume somebody up the stack will handle it, or they use a catch() and squash all errors).

While an unhandled return code might mean the code is in an unstable state, an unhandled exception often guarantees that the program will crash. Is this better?

While a return code is easily identifiable when writing code, it is often impossible (or just tediously time-consuming) to determine what exceptions might be thrown by a method you are calling. This typically results in a lot of very poor exception handling.

Exceptions are supposed to be used for "errors". Therein lies the difficulty. If a file is not found when you try to open it, is that an "error", or an "expected situation"? Only the caller knows. Using exceptions everywhere essentially elevates every piece of status information into an error.

Ultimately, error handling is something a programmer has to work at. This problem exists in both return codes and exceptions.

Thus, I use return codes for passing status information (including "warnings"), and exceptions for "serious errors". (and yes, sometimes it's hard to judge which category something falls under)

Example case from .net:

Int32.Parse throws exceptions (even though none of its exceptions are errors - it is up to the caller to verify the results and decide for themselves if the result is valid). And it's simply a pain (and a performance hit) to have to enclose every call to it in a try/catch. And if you forget to use a try/catch, a simple blank text entry field can crash your program.

Thus, Int32.TryParse() was born. This does the same thing, but returns an error code instead of an exception, so that you can simply ignore errors (accepting a default value of 0 for any illegal inputs). In many real life situations this is much cleaner, faster, easier and safer to use than Int32.Parse().

"TryParse" uses a naming convention to make it clear to the caller that errors might occur, that should be correctly handled. Another approach (to force programmers to handle errors better) is to make the return code into an out or ref parameter, so that the caller is explicitly made aware of the need to handle returned errors.

Jason Williams
The book Framework Design Guidelines 2nd Ed. gives guidelines on all of this, including the TryParse pattern and when to use it. **Highly** recommended reading.
TrueWill
this is a great answer
Jason
One major problem with this approach is that you then have two different styles and can end up with a lot of inconsistencies. In your example reason for using return codes, if you expect a file to exist when you go to open it and it's not there, yes that is an exception. Even if you guard it with a `File.Exists` test, the file open may still fail because the file was deleted in between the guard test and the open, in which case it is still an exception...
Scott Dorman
..."Using exceptions everywhere" elevates the responsibility of handling that exception condition to the *most appropriate* layer of code that can correctly do something about it. Yes, `Int32.Parse` throws exceptions because no matter which way you slice it trying to convert non-numeric string data to an actual integer is an error. `Int32.TryParse` was "born" because converting string data into numeric data is an extremely common UI pattern, the cost of the exception was too high given the volume, and makes a well defined test part of the method semantics. And `Int32.TryParse` can still throw.
Scott Dorman
What you're missing here is that my particular bit of code doesn't care if the file exists - in my context it is *not* an error. By prejudging this and forcing an everyday situaion to be treated as an error, you introduce a great deal of extra complexity, more code bloat, and a higher degree risk into the situation, without necessarily providing any benefit. The fopen() approach is that if the file can be opened, you get a file handle, and if it can't, you get a NULL - this is easier/less complex to handle, more efficient, and no less safe than an unhandled exception.
Jason Williams
TryParse throws exceptions for genuine errors, and leaves "validation difficulties" as the caller's responsibility. This leads to more efficient, easier to write, simpler and cleaner code that is a heck of a lot safer. (Of course, there is nothing to stop you calling parse in a situation where invalid inputs *are* an error. That is the beauty of using the "right tool for the job" instead of blindly throwing exceptions whenever you get nervous)
Jason Williams
(P.S. I'm not advocating total avoidance of exceptions, just saying that they should be used correctly. And there is a big grey area between "should be an exception" and "should be a status result")
Jason Williams
From your answer, "If a file is not found when you try to open it, is that an "error", or an "expected situation"? **Only the caller knows.**". This is exactly why the file open APIs in .NET throw an exception. `fopen()` returns `NULL` because there is nothing else it **can** return since C doesn't have exceptions. The problem with return codes is that they are very easy to ignore, accidentally or intentionally. Ignoring exceptions isn't as easy since it always requires explicit code to ignore them....
Scott Dorman
...Mixing return error codes and exceptions leads to more code bloat and spaghetti code than using exceptions alone and results in inconsitent code...When should I use a return code vs. an exception?
Scott Dorman
Ignoring an exception is just as easy as ignoring a return code - don't use a try/catch. Return codes are all around you - a method like 'bool HasChildren()' does not usually use an exception to tell you that the object doesn't have children - it uses a return code! But it could be argued that perhaps a lack of children on a node is indeed an error. So should it throw an exception if the result is 'false'? Of course not. Face it, you mix exceptions and return codes every day. The question when writing a method is: "should this informaiton be returned as an exception or a return code?"
Jason Williams