CODEWITHSUNDEEP
c#asp.netsqldatabaseexception
Ty.
Ty.

Reputation: 3948

Sanitizing Database Return Data

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;
}

Upvotes: 2

Views: 316

Answers (4)

Stephanie Page
Stephanie Page

Reputation: 3893

I would put your effort into making sure it cannot happen in the first place or minimizing the chances that it can.

For example from jonSkeet's post. If you expect a not-null condition for a column is there a constraint on that column? If no, you probably expect it because the DBA/DB developer told you it will be that way. I would tell them that you'll be relying on that fact and encourage, prod, cajole them into adding the not null constraint. If you're expect unique values, then ask for a unique constraint. Only uppercase characters, add a check constraint. There's little sense in your code checking for all uppercase and then the next app and the next app and the next app when it can be done once. Remember DRY -- Don't Repeat Yourself.

For the missing column error, my proactive approach is to make sure that there's an understanding by the db development team as to which tables and views and procedures are used by applications. Each application should have it's own username, each app's username should be granted select on only the tables it needs - not as a security feature but as documentation. If you change THIS table, it's used by these apps. Same with procs, grant execute to the apps which use it. If you keep this tight, and inculcate a "check for external dependencies when making changes" attitude, you'll head a lot of those errors off.

Upvotes: 0

AJ.
AJ.

Reputation: 13761

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!

Upvotes: 0

warren
warren

Reputation: 33435

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).

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1500175

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 :)

Upvotes: 4

Related Questions