Using unhandled exceptions instead of Contains()?

Question

Imagine an object you are working with has a collection of other objects associated with it, for example the Controls collection on a WinForm. You want to check for a certain object in the collection, but the collection doesn't have a Contains() method. There are several ways of dealing with this.

• Implement your own Contains() method by looping through all items in the collection to see if one of them is what you are looking for. This seems to be the "best practice" approach.
• I recently came across some code where instead of a loop, there was an attempt to access the object inside a try statement, as follows:

try  
{  
    Object aObject = myCollection[myObject];  
}  
catch(Exception e)  
{  
    //if this is thrown, then the object doesn't exist in the collection
}

My question is how poor of a programming practice do you consider the second option be and why? How is the performance of it compared to a loop through the collection?

Answer 1

If, while writing your code, you expect this object to be in the collection, and then during runtime you find that it isn't, I would call that an exceptional case, and it is proper to use an exception.

However, if you're simply testing for the existence of an object, and you find that it is not there, this is not exceptional. Using an exception in this case is not proper.

The analysis of the runtime performance depends on the actual collection being used, and the method if searching for it. That shouldn't matter though. Don't let the illusion of optimization fool you into writing confusing code.

Answer 2

The latter is an acceptable solution. Although I would definitely catch on the specific exception (ElementNotFound?) that the collection throws in that case.

Speedwise, it depends on the common case. If you're more likely to find the element than not, the exception solution will be faster. If you're more likely to fail, then it would depend on size of the collection and its iteration speed. Either way, you'd want to measure against normal use to see if this is actually a bottle neck before worrying about speed like this. Go for clarity first, and the latter solution is far more clear than the former.

Answer 3

I would have to think about it more as to how much I like it... my gut instinct is, eh, not so much...

EDIT: Ryan Fox's comments on the exceptional case is perfect, I concur

As for performance, it depends on the indexer on the collection. C# lets you override the indexer operator, so if it is doing a for loop like the contains method you would write, then it will be just as slow (with maybe a few nanoseconds slower due to the try/catch... but nothing to worry about unless that code itself is within a huge loop).

If the indexer is O(1) (or even O(log(n))... or anything faster than O(n)), then the try/catch solution would be faster of course.

Also, I am assuming the indexer is throwing the exception, if it is returning null, you could of course just check for null and not use the try/catch.

Answer 4

In general, using exception handling for program flow and logic is bad practice. I personally feel that the latter option is unacceptable use of exceptions. Given the features of languages commonly used these days (such as Linq and lambdas in C# for example) there's no reason not to write your own Contains() method.

As a final thought, these days most collections do have a contains method already. So I think for the most part this is a non-issue.

Answer 5

I would have to say that this is pretty bad practice. Whilst some people might be happy to say that looping through the collection is less efficient to throwing an exception, there is an overhead to throwing an exception. I would also question why you are using a collection to access an item by key when you would be better suited to using a dictionary or hashtable.

My main problem with this code however, is that regardless of the type of exception thrown, you are always going to be left with the same result.

For example, an exception could be thrown because the object doesn't exist in the collection, or because the collection itself is null or because you can't cast myCollect[myObject] to aObject.

All of these exceptions will get handled in the same way, which may not be your intention.

These are a couple of nice articles on when and where it is usally considered acceptable to throw exceptions:

• Foundations of Programming
• Throwing exceptions in c#

I particularly like this quote from the second article:

It is important that exceptions are thrown only when an unexpected or invalid activity occurs that prevents a method from completing its normal function. Exception handling introduces a small overhead and lowers performance so should not be used for normal program flow instead of conditional processing. It can also be difficult to maintain code that misuses exception handling in this way.

Answer 6

The general rule of thumb is to avoid using exceptions for control flow unless the circumstances that will trigger the exception are "exceptional" -- e.g., extremely rare!

If this is something that will happen normally and regularly it definitely should not be handled as an exception.

Exceptions are very, very slow due to all the overhead involved, so there can be performance reasons as well, if it's happening often enough.

Answer 7

Exceptions should be exceptional.

Something like 'The collection is missing because the database has fallen out from underneath it' is exceptional

Something like 'the key is not present' is normal behaviour for a dictionary.

For your specific example of a winforms Control collection, the Controls property has a ContainsKey method, which is what you're supposed to use.

There's no ContainsValue because when dealing with dictionaries/hashtables, there's no fast way short of iterating through the entire collection, of checking if something is present, so you're really discouraged from doing that.

As for WHY Exceptions should be exceptional, it's about 2 things

1. Indicating what your code is trying to do. You want to have your code match what it is trying to achieve, as closely as possible, so it is readable and maintainable. Exception handling adds a bunch of extra cruft which gets in the way of this purpose

2. Brevity of code. You want your code to do what it's doing in the most direct way, so it is readable and maintainable. Again, the cruft added by exception handling gets in the way of this.

Answer 8

Take a look at this blog post from Krzystof: http://blogs.msdn.com/kcwalina/archive/2008/07/17/ExceptionalError.aspx

Exceptions should be used for communicating error conditions, but they shouldn't be used as control logic (especially when there are far simpler ways to determine a condition, such as Contains).

Part of the issue is that exceptions, while not expensive to throw are expensive to catch and all exceptions are caught at some point.