views:

269

answers:

10

if I have a specific exception that I expect when it is going to occur; and to handle it for example I chose to display an error message upon its occurance, which would be better to do, and why?

Explanatory code:

try
{
    string result = dictionary[key];
}
catch (KeyNotFoundException e) 
{ 
    //display error
}

or:

if(!dictionary.ContainsKey(key))
{
    //display error
}
+3  A: 

The second approuch is better. Exception throwing can be very expensive.

astander
+11  A: 

Generally, exceptions are used to indicate exceptional conditions - something that would not normally occur, but your program still needs to handle gracefully (eg a file being inaccessible or readonly, a network connection going down). Normal control flow, like checking for a value in a dictionary, should not use exceptions if there is an equivalent function that has the same effect without using exceptions.

Having extra try/catch statements in the code also makes it less readable, and having exception handlers around a block of code puts certain limitations on the CLR that can cause worse performance.

In your example, if it is expected that the dictionary will have a certain key value, I would do something like

string result;
if (!dictionary.TryGetValue(key, out result)
{
    // display error
    return;   // or throw a specific exception if it really is a fatal error
}

// continue normal processing

This is a lot clearer than just having an exception handler round an element access

thecoop
Could you provide a source (or proof) for your last statement? I was not aware of this. My understanding was the the try/catch block was essentially free from a performance perspective as long as you don't throw an exception.
Steve
I remember it from a book, I'll need to check on monday. For one thing, the CLR doesn't inline methods that have exception handlers in them - http://blogs.msdn.com/davidnotario/archive/2004/11/01/250398.aspx
thecoop
@Steve: Yes, if you don't throw an exception there is only a negligible perf hit to enter/exit the try block. The issue is that if it is an expected condition that a key might not exist there are much better/simpler mechanisms that should be used (ContainsKey or TryGetValue).
Scott Dorman
A method needs to be 32 bytes or less of IL to even qualify for inlining. (see: http://blogs.msdn.com/ericgu/archive/2004/01/29/64717.aspx) - Any modestly complex routine is not going to get inlined.
Robert Venables
+2  A: 

The second approach is better for at least 3 reasons:

1) It is clearer. As a reader of your code, I expect an exception to indicate that something has gone wrong, even if it's handled.

2) When debugging with Visual Studio it's common to break on all exceptions, that makes it mildly annoying to deal with code which always throws an exception.

3) The second version is faster, but the effect is very small unless you are throwing many exceptions a second in a time-critical piece of code.

RossFabricant
+1  A: 

The second approach is better because throwing and hanlding exception has its performance hit. Throw rates above 100 per second are likely to noticeably impact the perfor- mance of most applications. Consider Exceptions and Performance.

Dzmitry Huba
+1  A: 

Exception handling is most useful when you need to provide an easy way out of a difficult situation - it can greatly simplify the code and decrease the potential for corner-case bugs.

It offers little advantage in very simple situations like this, and due to its performance penalty should not be used in such cases.

Artelius
A: 

It all depends on what you're application is doing and what the specific code is doing.

As thecoop says exceptions should be used for exceptional conditions. As an illustration take writing to a file. If checking for the existence of the file would slow your application down and/or the absence of the file is a serious problem then allow the exception to occur and trap that. If it's not so critical or recreating the file isn't a problem then do the file existence check first.

I've seen it argued that all error handling should be done via exceptions (most recently in Clean Code by Robert Martin) but I don't agree.

ChrisF
+7  A: 

Neither.

The second option is better than the first. As you expect this to happen normally, it's better to avoid the exception. Exceptions should preferrably only be used for exceptional situations, i.e. something that you can't easily predict and test for.

The best option however is the TryGetValue method, as it does both check and fetch:

if (dictionary.TryGetValue(key, out result)) {
   // use the result
} else {
   // display error
}
Guffa
A: 

Just a reminder. It is not an exception. An exception is something sounds like "no /etc on my UNIX machine". If you get wrong sense, you'll write wrong code shown as above.

EffoStaff Effo
Could you elaborate further on what you mean?
thecoop
"no /etc on my UNIX machine" - A well-known example about "is" or "is not" an exception when talking about exception handling. funny...
EffoStaff Effo
A: 

It depends greatly on what the dictionary is meant to do. If there is a high chance that the key will not be found because of program design, then you should do the trygetvalue. However, if the design of your program is such that not finding the key is an exceptional event, you should use the exception handler method.

Steve
+2  A: 

The second approach is probably better. Remember that exceptions are used for exceptional circumstances. Use this principle to guide your decision:

  • If you require that the key exists in the dictionary as an application invariant, then assume that it is there and deal with the exception if it isn't there.
  • If your application code doesn't require that the entry exist in the dictionary, then call ContainsKey() first.

My guess is that the latter is probably the correct course of action.

Disclaimer: I generally eschew the advice that performance should be the primary consideration here. Only let performance impact your decision once you have proven that you have a bottleneck! Anything before that is premature optimization and will lead to unnecessarily complication application code.

D.Shawley