It's time for another episode of Good Idea / Bad Idea:
Good Idea! Catching a specific exception from a suspect numeric conversion and showing a polite input error message.
Bad Idea! Catching all exceptions and "swallowing" the exception.
Okay, so it's not as funny as it is on Animaniacs, but true none-the-less.
Exception Handling in C#
In terms of exception handling I am a big fan of "catch it only if you can do something with it" and/or "catch it only if you mean it". Putting a try / catch around every method makes debugging very difficult and can leave your application in an unexpected state. Besides the code is less readable, more complex, and slower performing.
Before I get in to good practices, lets review some bad ones. The first and most obvious is throwing away exceptions (without regard to what they are). Example:
// Anti-pattern #1: Throw away the exception
private void button1_Click(object sender, EventArgs e) {
try {
AddPerson(new Person() { Name = txtName.Text });
} catch { }
}
Another example of what not to do is what I call the “catch just because” pattern. Example:
// Anti-pattern #2: Needless Try / Catch
private void DoStuff(string shipping) {
try {
// Call down to middle tier to do the shipping calculation
decimal ShippingCost = MyApi.Shipping.GetShippingCost(orderNum);
} catch (Exception ex) {
throw ex;
}
}
// Needless Try / Catch
private void button2_Click(object sender, EventArgs e) {
try {
// CalculateShipping has it's own Try / Catch; see below.
CalculateShipping();
} catch (Exception ex) {
MessageBox.Show(ex.Message);
}
}
There are multiple problems with the second example. First, there was no reason for “DoStuff” to catch the exception if all it was going to do was re-throw any exception it encountered. Second, “DoStuff” is catching any exception rather than limiting the scope to the kinds of exceptions it might expect like “Unknown Zipcode”, “Order not found”, etc. Next, the “button2_Click” is catching any exception and showing the raw message.
Rather than showing the exception message which is only part of the clues given in the exception object you should either show a user-friendly message indicating the kind of error that has taken place and save the exception information for later debugging. Alternatively, you can show ex.ToString() which will show you all information in the exception class. This is very unimpressive for end-user applications but can be acceptable with internal corporate apps and a simple screen shot will give you the information you need to find and take care of the problem.
Remember, an exception doesn’t necessarily mean that there is a bug in the software. If you do show a user an “unfriendly” exception message they will immediately think that the software has a bug when the issue could be as simple as the network is down or a database is temporarily unavailable. I always recommend taking the time to display user-friendly messages and saving the exception information elsewhere.
In the next section I will show some good examples on what "layer" to catch and/or re-throw an exception.
Presentation Layer
Personally I think that any event here should catch any/all expected exceptions and show them in a user-friendly manor. Notice I said "expected exceptions", this could be pretty vague unless you have an design pattern in place for exceptions in your applications so I'd start with that. One way to start is to make some "base" exceptions like "DataException", "ValidationException", "InputException", etc. More specific types of exceptions can be derived from these.
When do you make a new type of exception? Well, the exception should have the information needed to solve the problem. Stack traces and exception information are like clues; leave your self a breadcrumb trail to follow later. Throwing an ApplicationException because you couldn't connect to the database isn't very helpful.
It is also acceptable to use pre-defined / framework exception types when appropriate. For example:
public void DivideMyObjects(MyObjectType o1, MyOtherObjectType o2)
{
if (o1 == null || o2 == null)
throw new NullReferenceException(
"The numerator or the denominator cannot be null!");
if (o2.Value == 0)
throw new DivideByZeroException(
"Denominator may not be zero!");
if (o1.Value == null)
throw new ArgumentException(
"There is no value for the numerator");
...
}
You may notice in the code example above that a null reference may occur organically. I am a strong believer in organic exceptions as good practice. I define an organic exception as an error that is thrown by the Framework. The only exception to that rule is when there isn’t enough information from that exception to track down the problem. Remember, you need to leave yourself enough clues that you should be able to know what is wrong from the exception information given rather than expecting users to be able to remember what steps led up to the exception. Few, if any, end users will be able to help you reproduce any particular error. If this can’t be accomplished by an organic exception then you may want to re-throw that exception with the appropriate information and/or change the type of the exception to be more appropriate for the callers.
// Presentation Layer Good Example
private void Calculate_Shipping(object sender, EventArgs e) {
try {
decimal shippingCost = MyApi.Shipping.CalculateShipping(order);
txtShippingCost.Text = shippingCost.ToString("C");
} catch (UnknownZipcode uzipEx) {
// Specific
MessageBox.Show(
"Could not locate shipping information for this zip code!");
btnSave.Enabled = false;
} catch (ShippingException shipEx) {
// Less specific, but still helpful to the user.
MessageBox.Show("Unable to calculate shipping!"
+ Environment.NewLine +
shipEx.Message);
MyLogging.Log(shipEx);
} catch (Exception ex) {
MessageBox.Show(
"An unknown error has occured, Please report this error");
MyLogging.Log(ex);
Close();
}
}
In general you are pretty safe catching exceptions in the presentation layer. The only real thing to watch out for here is that your control states don’t become invalid because an exception is caught but the event still altered the state. For example, if a call to “Calculate_Shipping” throws an exception you will want to be careful to invalidate the shipping cost control and disable the “save order” button.
You will notice in the code above that I did catch generic exception. The presentation layer is usually okay doing this so long as it is taking adequate measures to protect application integrity. You will notice that I am closing the order form if I get an exception back that that is unknown. Of course this will give you plenty of motivation to make sure you know what kinds of exceptions are expected as end users will generally hate having their order forms close. You may be tempted to leave the form open or take lesser action but it is very important that you keep application integrity. Otherwise you will spend your life chasing down un-reproducible bugs.
Middle Layer
Most of your code should end up being middle layer code. We want the presentation layer to be as light as possible! The middle layer should act as the API of the application. Anything that the application “does” this layer should do it either directly or indirectly. In this sense the middle layer needs to be very up front in which exceptions are possible from any given call.
Take for example a method call in the middle layer that inserts a new customer into the database. Because you use the database objects you are exposed to any exceptions that are possible from the objects that you use such as SqlExceptions. You will either need to publish that you are going to allow SqlExceptions to be bubble through this API or you need to catch the expected exceptions from those objects and throw your own brand of exception.
Here is an example of publishing the exceptions that are known to be thrown:
// Middle Layer Exceptions
/// <summary>
/// Calculates shipping for a given order
/// </summary>
/// <param name="order">The order to calculate shipping for</param>
/// <returns>the shipping amount applied to the order</returns>
/// <exception cref="UnknownZipcode">Unknown Zip Code / Zip Not Found</exception>
/// <exception cref="BadWeightException">The weight of the order is zero or unknown</exception>
public decimal CalculateShipping(Order order) {
...
}
Simply using ndoc XML style comments will give the consumer of the API enough information to make good decisions regarding the use of exception handling from the intellisense.
Another thing you should avoid while designing your middle tier is the use of exceptions as “message handling” in non-exceptional cases. Remember, the reason you are throwing an exception is because something happened that was catastrophic enough that you were unable to continue with the normal flow of the method. I admit that there is still a lot of gray area here. Some of that gray area can be demonstrated in the sample code already shown. For example, rather than throwing an exception in the “CalculateShipping” method we could refactor so an exception wouldn't be necessary. Anytime this can be done cleanly I usually opt for these types of changes.
- Refactor to return a ShippingResult class that contains information about the calculation along with the rate. Such information could include whether it was able to successfully get a shipping rate.
- Create a ValidateShippingAddress method that looks up the address before the calculate shipping is run. It could return true if we know we can get shipping information for that address.
- We could pass back a nullable decimal result. A null return would indicate a failure to retrieve shipping information. We would want to make sure it was clear in our ndoc comments that this is why the value is nullable.
- We could pass pack a zero or negative result. Again, making sure this behavior is documented.
- We could implement the “Get Last Error” pattern; though at this point I’d rather see an exception.
Another question I am often asked is “When is it appropriate to re-throw an exception”. The answer to that questions is, of course, it depends. The next few sections will discuss some of these nuances of this question.
Bottom Layer
The bottom layer of an application could be a database layer, a Framework of some sort but the philosophy is very different from the middle layer. As the bottom layers deal with primitive classes and data and as such can stick pretty well to exception handling ideals. One of the big differences is that you won’t be using nearly as many user defined exception types. It is not taboo to use a user defined type in a Framework layer but it’s just likely to be unnecessary. The difficulty here is the exception message that you are going to pass along. There could be any number of middle layer calls that call your method so by nature these messages are going to be far more generic. Generic, however, does not mean it can’t be insightful. You should include as much information as possible about the exception and the data that caused the exception. Take the two following exception statements:
- “Invalid DateTime”
- “The date ‘4/11/2089’ is an invalid birth date! The date cannot be in the future!”
The first exception tells us that there is an invalid date time but other than a stack trace gives us no other information. The second exception is extremely helpful and specific, including as many clues as possible. The user may have entered the value ‘4/11/89’ and the computer may have just assumed the wrong century. The first error message would cause greater confusion whereas the second, if the user were to see that message, could be helpful.
Summary
Of course there are lots of different ideas and philosophies on exception handling so you need to do what makes sense for your project. Simply avoiding the two anti-patterns on the top of this blog will improve your code a lot. MY ONE LAST PEICE OF ADVIECE IS THAT SOMETIMES YOU REALLY WANT TO USE A DEBUG.ASSERT RATHER THAN AN EXCEPTION! The advantage of the assertion is that only debug builds show these messages. This is good for testing scenarios in which you want to be aware of a certain condition but it’s not necessarily an exception.
Links: