views:

97

answers:

2

I have a simple country table using a identity column for the primary key. There are also columns used to contain the 2 letter and 3 letter ISO-3166 Country Codes. Each of these columns is defined as a unique index.

On Inserts/Updates I want to simply notify the user if the ISO Code entered is already in use. I am relying on the database exception to trigger the process. I don't want to overwhelm the users with the technical details. I just want to tell him the the ISO value entered cannot be used.

Here is the T-SQL in the Update Stored Proc I've written. It seems long and prone to future bugs and/or continual maintenance as the app grows and things change. Is there a better or simpler way? I might be a bit wet behind the ears with .net, but I thought it would be easier.

BEGIN TRY
  UPDATE [Country] SET [CountryName] = @CountryName, [CountryISO] = @CountryISO, [CountryISO3] = @CountryISO3, [UpdateDate] = @updateDate WHERE (([CountryID] = @CountryID) AND ([RowVersion] = @Original_RowVersion));
END TRY

BEGIN CATCH
  DECLARE @ErrSeverity int, @ErrNumber int, @ErrLine int
  DECLARE @ErrMsg nvarchar(4000)
  SELECT @ErrSeverity = ERROR_SEVERITY(), @ErrNumber = ERROR_NUMBER(),@ErrState = ERROR_STATE(),
         @ErrMsg = 
            CASE WHEN ERROR_NUMBER() = 2601
            THEN 
              CASE 
              WHEN ISNULL(CHARINDEX('IX_COUNTRYISO3', ERROR_MESSAGE()), 0) > 0
                THEN 'The 3 letter ISO-3166 value entered is already in use. Please enter a unique 3 letter ISO-3166 value.'
              WHEN ISNULL(CHARINDEX('IX_COUNTRYISO', ERROR_MESSAGE()), 0) > 0
                THEN 'The 2 letter ISO-3166 value entered is already in use. Please enter a unique 2 letter ISO-3166 value.'
              ELSE 
                ERROR_MESSAGE() + '(SQL ErrNo: ' + CONVERT(varchar(50), ERROR_NUMBER()) + ')' 
              END 
            ELSE 
              ERROR_MESSAGE() + '(SQL ErrNo: ' + CONVERT(varchar(50), ERROR_NUMBER()) + ')'
            END;
  RAISERROR(@ErrMsg, @ErrSeverity, @ErrState)
END CATCH

Is my solution viable?
What is the best way to share this exception code between the Insert and Update StoredProcs?

I guess I am essentially asking for a code review.

Thank you very much Mike

A: 

Personally i would imagine that simply giving your unqique constraints/indexes sensible and verbose names would be a better option than this.

If users are interacting with the database directly then they should at lease have a scraping of IT knowledge.

If users are interacting via custom front end, then you should implement the error handling in the front end.

To catch it in the front end see this link, a basic example (and likely error ridden) example...

Catch (SqlExpcetion ex)
{
    if (e.Number == 2061 && ex.Message.Contains("IX_COUNTRYISO3")
    {
        MessageBox.Show("The 3 letter ISO-3166 value entered is already in use. Please enter a unique 3 letter ISO-3166 value.");
    }
    else
    {
         MessageBox.Show(String.Format("Error ({0}):{1}",ex.Number,ex.Message)
    }
}
Catch (Exception ex)
{
    MessageBox.Show(ex);
}
Paul Creasey
@Paul: thanks for your feedback. I don't know that I understand what you mean by verbose names. Does that mean I should give those unique constraints better names in the DB?Users are interacting through a custom front end. I couldn't figure out how to the SQL error number back through the exception class. I suppose I could just search the message string for 'Duplicate Key'. I need to deepen my understanding of the .net exception classes. Thanks
MikeSullivan
edited with a brief example of catching an sql exception.
Paul Creasey
I agree with Aaronaught though, you should not allow a user to even attempt this, the update should be validated before it is execuated.
Paul Creasey
+1  A: 

I agree, code like that is going to become a maintenance headache. There's nothing wrong with the code per se, it's just that the problem is being caught too late.

Unique constraints/indexes are a "last line of defense", at least in my world. If you want to provide a good user experience, you can't wait until the data is already being submitted to the database. You should be actively checking for duplicates at the UI level, and warning the user that s/he is about to submit a duplicate entry (and disabling the submission if duplicate keys are not allowed).

If a duplicate key does end up being submitted, that essentially describes a bug in the application logic, so it's OK for your database to be spitting out a "database-like" error message, you don't need to try to pretty it up. The application should try to handle such errors as gracefully as possible, as with any other unexpected exception.

If you're simply asking whether or not there's a way to modularize this kind of functionality in T-SQL... I don't think so. Not without an unholy mess of dynamic SQL, anyway.

Update - just saw a comment to the other answer, if you want to get the SQL error number back in C# then it's simple enough:

const int SqlDuplicateKeyError = 2601;

try
{
    db.Update(record);
}
catch (SqlException ex)
{
    switch (ex.Number)
    {
        case SqlDuplicateKeyError:
            // Custom error handling here
        default:
            throw;
    }
}
Aaronaught
@Aaronaught: Checking the values against the db prior to submission looks like a good strategy. Perhaps I will add an AJAX call when the values in either textbox changes. That will also allow me to use a more generic strategy for displaying and collecting exception info.Thank you for feedback. I will post back with any progress I make. It may not be for several days though.
MikeSullivan