C# Windows Forms - Best Practice Exception Handling?

Question

I'm currently in the process of writing my first Windows Forms application. I've read a few C# books now so I've got a relatively good understanding of what language features C# has to deal with exceptions. They're all quite theoretical however so what I haven't got yet is a feel for how to translate the basic concepts into a good exception-handling model in my application.

Would anyone like to share any pearls of wisdom on the subject? Post any common mistakes you've seen newbies like myself make, and any general advice on handling exceptions in a way that will my application more stable and robust.

The main things I'm currently trying to work out are:

• When should I re-throw an exception?
• Should I try to have a central error-handling mechanism of some kind?
• Do handling exceptions which might be thrown have a performance hit compared with pre-emptively testing things like whether a file on disk exists?
• Should all executable code be enclosed in try-catch-finally blocks?
• Are there any times when an empty catch block might be acceptable?

All advice gratefully received!

Answer 1

Exceptions are expensive but necessary. You don't need to wrap everything in a try catch but you do need to ensure that exceptions are always caught eventually. Much of it will depend on your design.

Don't re-throw if letting the exception rise will do just as well. Never let errors pass unnoticed.

example:

void Main()
{
  try {
    DoStuff();
  }
  catch(Exception ex) {
    LogStuff(ex.ToString());
  }

void DoStuff() {
... Stuff ...
}

If DoStuff goes wrong you'll want it to bail anyway. The exception will get thrown up to main and you'll see the train of events in the stack trace of ex.

Answer 2

Hi Jon,

I like the philosophy of not catching anything I don't intend on handling, whatever handling means in my particular context.

I hate it when I see code such as:

try
{
   // some stuff is done here
}
catch
{
}

I have seen this from time to time and it is quite difficult to find problems when someone 'eats' the exceptions. A coworker I had does this and it tends to end up being a contributor to a steady stream of issues.

I re-throw if there is something that my particular class needs to do in response to an exception but the problem needs to be bubbled out to however called the method where it happened.

I think code should be written proactively and that exceptions should be for exceptional situations, not to avoid testing for conditions.

Answer 3

I'm just on my way out but will give you a brief run down on where to use exception handling. I will attempt to address your other points when I return :)

1. Explicitly check for all known error conditions*
2. Add a try/catch around the code if your are unsure if you were able to handle all cases
3. Add a try/catch around the code if the .NET interface you are calling throws an exception
4. Add a try/catch around the code if it crosses a complexity threshold for you
5. Add a try/catch around the code if for a sanity check: You are asserting THIS SHOULD NEVER HAPPEN
6. As a general rule, I do not use exceptions as a replacement for return codes. This is fine for .NET, but not me. I do have exceptions (hehe) to this rule though, it depends on the architecture of the application your are working on as well.

*Within reason. There is no need to check to see if say a cosmic ray hit your data causing a couple bits to get flipped. Understanding what is "reasonable" is an acquired skill for an engineer. It's hard to quantify, yet easy to intuit. That is, I can easily explain why I use a try/catch in any particular instance, yet I am hard pressed to imbue another with this same knowledge.

I for one tend to steer away from heavily exception based architectures. try/catch doesn't have a performance hit as such, the hit comes in when the exception is thrown and the code might have to walk up several levels of the call stack before something handles it.

Answer 4

When should I re-throw an exception?

Everywhere, but end user methods... like button click handlers

Should I try to have a central error-handling mechanism of some kind?

I write a log file... pretty easy for a WinForm app

Do handling exceptions which might be thrown have a performance hit compared with pre-emptively testing things like whether a file on disk exists?

I'm not sure about this, but I believe it is a good practice to thow exceptions... I mean you can ask whether a file exists and if it doesn't throw a FileNotFoundException

Should all executable code be enclosed in try-catch-finally blocks?

yeap

Are there any times when an empty catch block might be acceptable?

Yes, let's say you want to show a date, but you have no clue how that date was stores (dd/mm/yyyy, mm/dd/yyyy, etc) you try tp parse it but if it fails just keep going... if it is irrelevant to you... I would say yes, there is

Answer 5

The one thing I learned very quickly was to enclose absolutely every chunk of code that interacts with anything outside the flow of my program (i.e. File System, Database Calls, User Input) with try-catch blocks. Try-catch can incur a performance hit, but usually in these places in your code it won't be noticeable and it will pay for itself with safety.

I have used empty catch-blocks in places where the user might do something that isn't really "incorrect", but it can throw an exception...an example that comes to mind is in a GridView if the user DoubleCLicks the gray placeholder cell on the top-left it will fire the CellDoubleClick event, but the cell doesn't belong to a row. In that case, you dont really need to post a message but if you don't catch it it will throw an unhandled exception error to the user.

Answer 6

There is an excellent code CodeProject article here. Here are a couple of highlights:

• Plan for the worst*
• Check it early
• Don't trust external data
• The only reliable devices are: the video, the mouse and keyboard.
• Writes can fail, too
• Code Safely
• Don't throw new Exception()
• Don't put important exception information on the Message field
• Put a single catch (Exception ex) per thread
• Generic Exceptions caught should be published
• Log Exception.ToString(); never log only Exception.Message!
• Don't catch (Exception) more than once per thread
• Don't ever swallow exceptions
• Cleanup code should be put in finally blocks
• Use "using" everywhere
• Don't return special values on error conditions
• Don't use exceptions to indicate absence of a resource
• Don't use exception handling as means of returning information from a method
• Use exceptions for errors that should not be ignored
• Don't clear the stack trace when re-throwing an exception
• Avoid changing exceptions without adding semantic value
• Exceptions should be marked [Serializable]
• When in doubt, don't Assert, throw an Exception
• Each exception class should have at least the three original constructors
• Be careful when using the AppDomain.UnhandledException event
• Don't reinvent the wheel
• Don't use Unstructured Error Handling (VB.Net)

Answer 7

A few more bits ...

You absolutely should have a centralized exception handling policy in place. This can be as simple as wrapping Main() in a try/catch, failing fast with a graceful error message to the user. This is the "last resort" exception handler.

Preemptive checks are always correct if feasible, but not always perfect. For example, between the code where you check for a file's existence and the next line where you open it, the file could have been deleted or some other issue may impede your access. You still need try/catch/finally in that world. Use both the preemptive check and the try/catch/finally as appropriate.

Never "swallow" an exception, except in the most well-documented cases when you are absolutely, positively sure that the exception being thrown is livable. This will almost never be the case. (And if it is, make sure you're swallowing only the specific exception class -- don't ever swallow System.Exception.)

When building libraries (used by your app), do not swallow exceptions, and do not be afraid to let the exceptions bubble up. Do not re-throw unless you have something useful to add. Do not ever (in C#) do this:

throw ex;

As you will erase the call stack. If you must re-throw (which is occasionally necessary, such as when using the Exception Handling Block of Enterprise Library), use the following:

throw;

At the end of the day, the very vast majority of exceptions thrown by a running application should be exposed somewhere. They should not be exposed to end users (as they often contain proprietary or otherwise valuable data), but rather usually logged, with administrators notified of the exception. The user can be presented with a generic dialog box, maybe with a reference number, to keep things simple.

Exception handling in .NET is more art than science. Everyone will have their favorites to share here. These are just a few of the tips I've picked up using .NET since day 1, techniques which have saved my bacon on more than one occasion. Your mileage may vary.

Answer 8

Here are a few guidelines that I follow

1. Fail-Fast: This is more of a exception generating guideline, For every assumption that you make and every parameter that you are getting into a function do a check to make sure that you're starting off with the right data and that the assumptions you're making are correct. Typical checks include, argument not null, argument in expected range etc.

2. When rethrowing preserve stack trace - This simply translates to using throw when rethrowing instead of throw new Exception(). Alternatively if you feel that you can add more information then wrap the original exception as an inner exception. But if you're catching an exception only to log it then definitely use throw;

3. Do not catch exceptions that you cannot handle, so don't worry about things like OutOfMemoryException because if they occur you won't be able to do much anyways.

4. Do hook global exception handlers and make sure to log as much information as possible. For winforms hook both the appdomain and thread unhandled exception events.

5. Performance should only be taken into consideration when you've analyzed the code and seen that it's causing a performance bottleneck, by default optimize for readability and design. So about your original question on the file existence check, I would say it depends, If you can do something about the file not being there, then yes do that check otherwise if all you're going to do is throw an exception if the file's not there then I don't see the point.

6. There are definitely times when empty catch blocks are required, I think people who say otherwise have not worked on codebases that have evolved over several releases. But they should be commented and reviewed to make sure that they're really needed. The most typical example is developers using try/catch to convert string to integer instead of using ParseInt().

7. If you expect the caller of your code to be able to handle error conditions then create custom exceptions that detail what the un excepected situation is and provide relevant information. Otherwise just stick to built-in exception types as much as possible.

Answer 9

When re-throwing an exception the key word throw by it self. This will throw the caught exception and still will be able to use stack trace to see where it came from.

Try
{
int a = 10 / 0;
}
catch(exception e){
//error logging
throw;
}

doing this will cause the stack trace to end in the catch statement. (avoid this)

catch(Exception e)
// logging
throw e;
}

Answer 10

The golden rule that have tried to stick to is handle the exception as close to the source as possible.

If you must re-throw an exception try to add to it, re-throwing a FileNotFoundException does not help much but throwing a ConfigurationFileNotFoundException will allow it to be captured and acted upon somewhere up the chain.

Another rule I try to follow is not to use try/catch as a form of program flow, so I do verify files/connections, ensure objects have been initiated, ect.. prior to using them. Try/catch should be for Exceptions, things you can not control.

As for an empty catch block, if you are doing anything of importance in the code that generated the exception you should re-throw the exception at a minimum. If there is no consequences of the code that threw the exception not running why did you write it in the first place.

Answer 11

All of the advice posted here so far is good and worth heeding.

One thing I'd like to expand on is your question "Do handling exceptions which might be thrown have a performance hit compared with pre-emptively testing things like whether a file on disk exists?"

The naive rule of thumb is "try/catch blocks are expensive." That's not actually true. Trying isn't expensive. It's the catching, where the system has to create an Exception object and load it up with the stack trace, that's expensive. There are many cases in which the exception is, well, exceptional enough that it's perfectly fine to wrap the code in a try/catch block.

For instance, if you're populating a Dictionary, this:

try
{
   dict.Add(key, value);
}
catch(KeyException)
{
}

is often faster than doing this:

if (!dict.ContainsKey(key))
{
   dict.Add(key, value);
}

for every single item you're adding, because the exception only gets thrown when you are adding a duplicate key. (LINQ aggregate queries do this.)

In the example you gave, I'd use try/catch almost without thinking. First, just because the file exists when you check for it doesn't mean that it's going to exist when you open it, so you should really handle the exception anyway.

Second, and I think more importantly, unless your a) your process is opening thousands of files and b) the odds of a file it's trying to open not existing are not trivially low, the performance hit of creating the exception is not something you're ever going to notice. Generally speaking, when your program is trying to open a file, it's only trying to open one file. That's a case where writing safer code is almost certainly going to be better than writing the fastest possible code.

Answer 12

n my experience I've seen fit to catch exceptions when I know I'm going to be creating them. For instances when I'm in a web application and I'm doing a Response.Redirect, I know I'll be getting a System.ThreadAbortException. Since it's intentional I just have a catch for the specific type and just swallow it.

try
{
/*Doing stuff that may cause an exception*/
Response.Redirect("http:\\www.somewhereelse.com");
}
catch (ThreadAbortException tex){/*Ignore*/}
catch (Exception ex){/*HandleException*/}

Answer 13

I deeply agree the rule of:

• Never let errors pass unnoticed.

The reason is that:

• When you first write down the code, most likely you won't have the full knowledge of 3-party code, .NET FCL lirary, or your co-workers latest contributions. In reality you cannot refuse to write code until you know every exception possiblity well. So
• I constanly find that I use try/catch(Exception ex) just because I want to protected myself from unknown things, and, as you noticed, I catch Exception, not the more specific such as OutOfMemoryException etc. And, I always make the exception being popped up to me(or the QA) by ForceAssert.AlwaysAssert(false, ex.ToString() );

ForceAssert.AlwaysAssert is my personal way of Trace.Assert just regardless of whether the DEBUG/TRACE macro is defined.

The development cycle maybe: I noticed the ugly Assert dialog or someone else complain to me about it, then I come back to the code and figure out the reason to raise the exception and decide how to process it.

By this way I can write down MY code in a short time and protected me from unknown domain, but always being noticed if the abnormal things happened, by this way the system got safe and more safety.

I know many of you wont agree with me because a developer should known every detail of his/her code, frankly, I'm also a purist in the old days. But nowdays I learned that the above policy is more pragmatic.

For WinForms code, a golden rule I always obey is:

• Always try/catch(Exception) your event handler code

this will protected your UI being always usable.

For performance hit, performance penalty only happens when the code reachs catch, executing try code without the actual exception raised has no significant effect.

Exception should be happened with little chance, otherwise it's not exceptions.

Answer 14

Note that Windows Forms has it own exception handling mechanism. If a button in form is clicked and its handler throws an exception which isn't caught in the handler, Windows Forms will display its own Unhandled Exception Dialog.

To prevent the Unhandled Exception Dialog to be displayed and catch such exceptions for logging and/or for providing your own error dialog you can attach to the Application.ThreadException event before the call to Application.Run() in your Main() method.