Sanitizing Database Return Data

Question

I am wondering what everyone thinks the best method of handling results from your own database is. Other teams may be involved and there is always the chance the procedure/data could be altered and erroneous results would occur. My question is this. Is it better to let and exception occur, catch and log it or try to handle all contingencies and hide the error? Say, something like below.

if (dr.Table.Columns.Contains("column") && !dr["column"].Equals(DBNull.Value))
{
    this.value = (type)dr["column"];
}
else
{
   this.value= null;
}

Answer 1

Personally I like failing fast - with an appropriately apologetic user message, of course. There are some things it's worth recovering from, but something like a column you expect to be non-null being null sounds more significant to me.

Of course, I'd also try to set up some smoke tests to make sure you find out about it before production :)

Answer 2

Database constraints should be taking care of most of this for you. For what isn't, I would suggest going back to the db design and fixing those constraints.

Failing that, returning an exception on that field would be best so that the data can be fixed (or removed).

Answer 3

Handle everything you think is worth handling and catch exceptions if something "impossible" happens.

As @AviewAnew says, you are perhaps being a little paranoid, but that doesn't mean you're wrong!

I would have thought that a column missing would be much worse than a null value, so how about throwing an exception for a missing column?

try // wrap everything in a try/catch to handle things I haven't thought of
{

    if ( !dr.Table.Columns.Contains("column") )
    {
        throw new SomeSortOfException("cloumn: " + column + " is missing" );
    }
    else // strictly don't need the else but it makes the code easier to follow
    {
        if (dr["column"].Equals(DBNull.Value))
        {
            this.value= null;
        }
        else
        {
            this.value = (type) dr["column"];
        }
    }
}
catch( SomeSortOfException ex )
{
    throw;
}
catch( Exception ex )
{
    // handle or throw impossible exceptions here
}

On the other hand... if you're going to be putting all these checks throughout your code, the maintenance overhead is going to be considerable. ... it's another thing to consider.

Your call!