Shortly after finishing a list of a dozen things I hate, I settled into work to find a bug that has brought an application to its knees. At first, we believed that maybe the underlying data had changed, and that as a result queries that used to match on an exact date & time at midnight no longer did.

However, further investigation revealed something much more stupid and avoidable than this.

Let's start at the beginning. Take a look at this little gem:

try {
        DataTable dt = someFooClass.GetSomeData(dateParam);
        ds.Tables.Add(dt);
} catch (Exception ex) {
        string bitBucket = ex.Message + " " + ex.StackTrace;
}
return dt;

For everyone who has ever done this practice of tossing your errors into a black hole, including me, go back to programming school, YOU SUCK!

If you are doing something like this anywhere in your code, STOP! Remove them. And, no I dont care if it breaks your application. You will be better off after you either tighten the exception catching to grab only the error you want (and/or rethrow everything else), or better yet fix the problem that is causing the exception in the first place.

But why? Seems harless enough, right? After all we're only calling one function and [probably] that function is very simple and has no special logic or its own calls are proven to be stable. So why the fuss?

Well. Let's look at what boiled up from inside the GetSomeData function after I deleted the exception flypaper:

//double result = numberValue1 + numberValue2;
double result = Convert.ToDouble(IsNull(numberValue1.ToString() + numberValue2.ToString()));

Here are my observations about this little peice of code genius.

1. If numberValue1 and numberValue2 were numbers to begin with, then why do we convert them into strings and then back into a number again? This seems hopelessly convoluted and prone to potential errors. For those of you who have not been coding for very long, this is called "walking three sides around the barn". Programmers hate unecessary work, and chances are good (though there are exceptions) that the framework developers have provided you some better way of getting what you want with less effort.
2. It seems pretty obvious that the programmer here did not mean to concatinate two number-like strings together and then parse them back into a number later. Ironically, this will work (producing meaningless garbage data), if at least one of the two inputs has no decimal places and numberValue two is positive.
3. If testing for null is required, it should've been done on both the inputs and not on their sum.
4. It also seems pretty obvious that someone cane along and patched this code to handle null or System.DBNull that was coming in from the database or elsewhere in an unanticipated way. As a result, what used to be simple arithmatic is now calling a custom function.
5. IsNull has a misleading name. In this case IsNull returns a double, not a boolean and does some parsing to the values to return zero if they are null. Further, IsNull is a name pattern you will find in code a lot, and people generally assume they know what it means. So, if your code does not follow the same kind of logic as pther code with that name, use a different name. I would suggest NullsAsZero or ZeroNulls as alternative names.
6. This patch was buried under exception catching and as such it was never really adequately tested. Since it was developed in a hurry, I doubt anyone knew that it was broken and it may have returned seemingly valid results that confused the crap out of everyone in UAT later on.
7. The end result here is that what probably took five minutes to write (badly) took a week to find and fix. That is not a good trade.

Microsoft has good resources to help you understand when you should and should not mask exceptions and how they can be handled at different layers of your applications. (Links will follow when I feel like going to look them up.) In the mean time, just keep this one simple rule in mind:

If you don't understand best practices for exception handling, or you are not putting any meaningful handling logic inside your "catch" block, then DON'T catch the exception in the first place.

Update: Thanks to source control, we found the author of this nice little peice of "new math". (As my colleague said "1 + 1 = 11?"... heh) It's so refreshing to work in a place where public humiliation is used as a method to improve coding standards. :->