Inner Drive Software

We're still thrashing through the abandoned application, in which we found this gem:

public class ClsMonthName
{
	public ClsMonthName()
	{
		//
		// TODO: Add constructor logic here
		//
	}
	public string getMonthName(int month)
	{
		string monthName="";
		switch (month)
		{
			case 1:
				monthName="January";
				break;
				
			/* etc. */
			
			case 12:
				monthName="December";
				break;
		}
		return monthName;
	}
}

What's wrong with this picture? Oh, where to start.

First, in .NET, the DateTime.ToString(string) method will pretty-print just the month portion of any date. So whoever wrote this class didn't know that very-basic fact about the .NET Framework.

Second, forgetting that the method is completely unnecessary, why is it a member method instead of just a static method? I mean, why bother instantiating an entire class—in some places I found, at the module level—when it has no side-effects or data?

Third, where's the default case? Sure, as implemented in the coding disaster they delivered, the month parameter always traces back to a DateTime.Month value. But it's a public method of a public class, callable throughout the application. What if month contains one of the other 4,395,630,580 valid values of Int32? Oops.

Or even more fun, what if DateTime.Month returns 13? If you think that can't happen, check out the System.Globalization.HebrewCalendar class, which defines 13 months during leap years (which happen about every other year). But then again, if the user is using the Hebrew calendar, translating the months into English will probably yield unpredictable results.

Finally, most of the code that called this method used the month name to disambiguate SQL statements. In other words, the developer ran into trouble because his dates were confusing SQL Server. But the fact that he was generating SQL statements on the fly was already a yellow flag that suggested deeper problems with the code.

All part of the fun of cleaning up someone else's mess.