Thursday, January 03, 2008

C# WTF: Abuse of exception handlers

I've started collecting assorted examples of bad code, and will start periodically posting them. Everything seen here was first seen in real code (and fixed, when possible). Just remember, these are examples of what you shouldn't ever do! :-)

Type checking by InvalidCastException:

Common uses:
  • Checking the type of an object in a collection when the developer does not know about the "is" operator or its equivalents.


Selection selItems = UtilServices.GetInstance().GetSelectedItems();

if (selItems != null)

{

    try

    {

        foreach (object item in selItems)

        {

            // Cast to a Foo Item.

            FooItem fooItem = (FooItem)item;

        }

        ProcessSuccess();

    }

    catch (System.Exception ex)

    {

        // If an exception is thrown, the items are not FooItems

        Logger.Error("Exception: " + ex.Message);

        ProcessFailure();

    }

}



Null checking by Exception handler:

Common uses:
  • Not being bothered by the hassle of all those "if" statements.


try

{

    foo = bar.GetSomeData();

    thing = foo.ThingValue;

}

catch

{

    foo = bar.GetOtherData();

    thing = foo.ThingValue;

}

1 comment:

Makoto Schoppert said...

Those are pretty rough samples and I've see my fair share of abusive use of exception catching.

LINQ makes it really easy to check if all the items in a collection are of a certain type. Here's how to do it in one line:

bool areAllItmesFooItem = selItems.All(i => i is FooItem);