Best Practice for Exception Handling in a Windows Forms Application?

Question

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.

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)

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.

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.