A common pattern is "global error handlers"- functions you call in your catch block that do some task, usually logging, when an exception occurs. Given that exception handling frequently involves a fair bit of repeated code, centralizing that isn't a terrible idea. So when Magnus found code like this in the codebase, he didn't think much of it:

public List<ErrandComboClass> GetErrandCombo()
{
    List<ErrandComboClass> list = null;
    try
    {
        using (MyEntities my = new MyEntities())
        {
            list = my.Errand.Select(x => new ErrandComboClass { ID = (int)x.Id, Name = x.Dnr }).OrderBy(x => x.Name).ToList();
        }
    }
    catch (Exception ex)
    {
        ExceptionHelper.CatchError(ex);
    }

    return list;
}

I can't say I love the naming conventions, but it's a pretty straight forward C# Linq method, projecting and sorting data. I don't think that the ToList should be there, because we could be leveraging lazy evaluation (or maybe we shouldn't, I don't know their access patterns, but at a guess, lazy would be better here). Anything goes wrong, we pass the error to our global exception handler.

This exception handler was called everywhere. So what did it do?

public static class ExceptionHelper
{
    public static void CatchError(Exception ex)
    {
        string s = ex.ToString();
    }       
}

Oh, nothing. It does (basically) nothing. Presumably, once upon a time, this was meant to be an error handling framework that they'd hang a lot of functionality off of- but that never happened. This was their production code, and this was their error handling solution.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!