Always Check Your Datatypes!

I built and maintain several web apps for personal and family use, one of which is a budget management tool. During a recent (major) refactoring and update, I noticed one particular error with the notification system:

Yep, "CategoryName" is not an actual category - it's a database column name. So how did this get through to production?

Obviously this was missed during testing, but debugging the relevant code wasn't super-obvious either. What do you see wrong?

expense.UserID = SQLiteHelper.GetID(reader["UserID"]);
expense.CategoryID = SQLiteHelper.GetID(reader["CategoryID"]);
expense.CategoryName = SQLiteHelper.GetString("CategoryName");
expense.Amount = SQLiteHelper.GetDecimalValue(reader["Amount"]);

And again, yep, it was as simple as an incorrect reference. The third line is missing the reader object; it should read:

expense.CategoryName = SQLiteHelper.GetString(reader["CategoryName"]);

The GetString method work with both inputs - it technically passed unit and functional tests, so the problem wasn't obvious. The implementation tells the story:

public static string GetString(object dbValue)
{
    if (dbValue != null && dbValue != DBNull.Value)
        return dbValue.ToString() ?? string.Empty;

    return string.Empty;
}

Using object as a parameter type is too vague here. There should be another check of the object type to ensure we're working with a SQLite database object rather than a simple string. D'oh!

The lesson learned? Be explicit when expecting certain datatypes rather than letting methods like ToString() coalesce types.