0

I currently have the following code to detect a duplicate entry, I suppose there is a cleaner way, but I have yet to find it.... can anyone guide me if this is the correct way?

catch (DbUpdateException e)
{
    if (e.InnerException != null)
       if (e.InnerException is UpdateException)
          if (e.InnerException.InnerException != null)
             if (e.InnerException.InnerException is SqlException)
             {
                 SqlException ex = e.InnerException.InnerException as SqlException;

                 if (ex.Number == 2601)
                 {
                     ModelState.AddModelError("", "Unit number already exists");
                 }
             }
         }
    }
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Martin
  • 397
  • 3
  • 16
  • It could use a bit of code cleanup to make it nicer, but letting the DB's foreign keys to catch such errors are a great way and possibly the most efficient too. – Alejandro Apr 11 '15 at 19:07

2 Answers2

4

Call Exception.GetBaseException(), this lets you get to the innermost exception in much less code.

catch (DbUpdateException e)
{
    var ex = e.GetBaseException() as SqlException;

    if (ex != null && ex.Number == 2601)
    {
        ModelState.AddModelError("", "Unit number already exists");
    }
    else
    {
         //The exception was some other kind we weren't expecting
         //Let the exception bubble.
         throw;
    }
}

If the exception you are interested in is not the base exception or you want to insure that the layer at e.InnerException is a UpdateException and not some other type your code can still be simplified a lot by removing redundant code and doing more than one check per if.

catch (DbUpdateException e)
{
    bool handled = false;

    if (e.InnerException != null && e.InnerException is UpdateException)
    {
        var ex = e.InnerException.InnerException as SqlException;
        if (ex != null && ex.Number == 2601)
        {
            ModelState.AddModelError("", "Unit number already exists");
            handled = true;
        }
     }

     //The exception was some other kind we weren't expecting
     //Let the exception bubble.
     if(!handled)
         throw;
}
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • As I have most of my insert logic moved to my BLL, Im now wondering where I should capture the exceptions? Most logically is the UI layer as I will update the UI based on the exception, however it incurs a lot of repetitive error catching. Also I have a UoW so I call a single SaveChanges(). Can I clean up this repetitive code to an easier way to check for uniqueness? – Martin Apr 19 '15 at 19:37
1

It is not recommended to use Exceptions to write application logic.

Why don't you just check in your DB through EF if the unit no. exists. If it exists, then display a validation error and if not then proceed with the insert operation.

Just ensure that your unique column is indexed.

Praveen Paulose
  • 5,741
  • 1
  • 15
  • 19
  • 1
    Doing so creates chances for race conditions in which someone else inserts the record between your check and insert. Relying on the foreing keys is the way to go, and failures there are reflected though exceptions. – Alejandro Apr 11 '15 at 19:09
  • You can manage the Exceptions for these race conditions, but I still would not recommend to use it as application logic. – Praveen Paulose Apr 11 '15 at 19:10
  • At that point you're basically back at the original OP code to check as a last resort, only implementing a meaningless and inefficient check beforehand. This is one of the situation where letting the DB explode and catching the exception makes sense, as long as there is a foreign key in place. – Alejandro Apr 11 '15 at 19:13
  • No, it is not a meaningless check. Exception handling has substantial overheads involved. read this http://stackoverflow.com/questions/161942/how-slow-are-net-exceptions. And try this code on 1000 records, where all 1000 exist in DB. – Praveen Paulose Apr 11 '15 at 19:16
  • 1
    Exceptions are slow yes, but only relative to other C# code. The checking involves doing a second round trip to the server (with all associated network costs) and those are much slower that just catching an exception. – Alejandro Apr 11 '15 at 19:20
  • BTW, I correct myself, it should have been **primary key** or **unique constraint**, NOT **foreign key** :D – Alejandro Apr 11 '15 at 19:21
  • This is what I had before, but was cleaning up database calls that were not required. I will only move back to this solution once I implement AJAX calls from the webform to check in-line, for now I just wanted to use the exception as the case will be quite rare. I'm using an unique index on this column. – Martin Apr 12 '15 at 07:27