best way to catch database constraint errors

Question

I am calling a stored procedure that inserts data in to a sql server database from c#. I have a number of constraints on the table such as unique column etc. At present I have the following code:

try
{
   // inset data
}
catch (SqlException ex)
{
    if (ex.Message.ToLower().Contains("duplicate key"))
    {

        if (ex.Message.ToLower().Contains("url"))
        {
            return 1;
        }

        if (ex.Message.ToLower().Contains("email"))
        {
            return 2;
        }
    }

    return 3;
}

Is it better practise to check if column is unique etc before inserting the data in C#, or in store procedure or let an exception occur and handle like above? I am not a fan of the above but looking for best practise in this area.

Answer 1

I view database constraints as a last resort kind of thing. (I.e. by all means they should be present in your schema as a backup way of maintaining data integrity.) But I'd say the data should really be valid before you try to save it in the database. If for no other reason, then because providing feedback about invalid input is a UI concern, and a data validity error really shouldn't bubble up and down the entire tier stack every single time.

Furthermore, there are many sorts of assertions you want to make about the shape of your data that can't be expressed using constraints easily. (E.g. state transitions of an order. "An order can only go to SHIPPED from PAID" or more complex scenarios.) That is, you'd need to use involving procedural-language based checks, ones that duplicate even more of your business logic, and then have those report some sort of error code as well, and include yet more complexity in your app just for the sake of doing all your data validation in the schema definition.

Validation is inherently hard to place in an app since it concerns both the UI and is coupled to the model schema, but I veer on the side of doing it near the UI.

Answer 2

I see two questions here, and here's my take...

Are database constraints good? For large systems they're indepensible. Most large systems have more than one front end, and not always in compatible languages where middle-tier or UI data-checking logic can be shared. They may also have batch processes in Transact-SQL or PL/SQL only. It's fine to duplicate the checking on the front end, but in a multi-user app the only way to truly check uniqueness is to insert the record and see what the database says. Same with foreign key constraints - you don't truly know until you try to insert/update/delete.

Should exceptions be allowed to throw, or should return values be substituted? Here's the code from the question:

    try
    {
       // inset data
    }
    catch (SqlException ex)
    {
        if (ex.Message.ToLower().Contains("duplicate key"))
        {
            if (ex.Message.ToLower().Contains("url"))
            {
                return 1; // Sure, that's one good way to do it
            }
            if (ex.Message.ToLower().Contains("email"))
            {
                return 2; // Sure, that's one good way to do it
            }
        }
        return 3; // EVIL! Or at least quasi-evil :)
    }

If you can guarantee that the calling program will actually act based on the return value, I think the return 1 and return 2 are best left to your judgement. I prefer to rethrow a custom exception for cases like this (for example DuplicateEmailException) but that's just me - the return values will do the trick too. After all, consumer classes can ignore exceptions just as easily as they can ignore return values.

I'm against the return 3. This means there was an unexpected exception (database down, bad connection, whatever). Here you have an unspecified error, and the only diagnostic information you have is this: "3". Imagine posting a question on SO that says I tried to insert a row but the system said '3'. Please advise. It would be closed within seconds :)

If you don't know how to handle an exception in the data class, there's no way a consumer of the data class can handle it. At this point you're pretty much hosed so I say log the error, then exit as gracefully as possible with an "Unexpected error" message.

I know I ranted a bit about the unexpected exception, but I've handled too many support incidents where the programmer just sequelched database exceptions, and when something unexpected came up the app either failed silently or failed downstream, leaving zero diagnostic information. Very naughty.