views:

141

answers:

3

Is it acceptable to do this? First try to the add the entity. If the add fails, it doesn't matter because that means the entity already exists?

Or is there a more elegant/easy solution?

EntityFrameworkEntities dal = EntityDataModelHelper.GetEntityDataModel();

try
{
    dal.AddToXXXXXX(xxxxxxx);
}
catch
{

}

try
{
    dal.SaveChanges();
    return true;
}
catch
{
    return false;
}

OK I shortened it to...

EntityFrameworkEntities dal = EntityDataModelHelper.GetEntityDataModel();

if(xxxxxxx.ID == 0)
{
    dal.AddToXXXXXX(xxxxxxx);
}

try
{
    dal.SaveChanges();
    return true;
}
catch
{
    return false;
}
+5  A: 

It is certainly not OK to do this. A catch statement with no type in C# means "catch any standard or non-standard exception". But you're intent is to prevent a duplicate Add. Adds can fail for a variety of reasons which are not indicative of an existing entry. For example, that method could throw a null reference and you'd assume it was added.

If you want to check for a duplicate add, you must catch only the exception that is thrown for a duplicate add.

JaredPar
A: 

You could replace the first Try-Catch with an If statement, I think you'll still want the second one though.

Edit: It's also not recommended to just catch all exceptions in one block without regard to what they are.

P.S. Try Catch blocks use more processing power (time) than If statements.

Lucas McCoy
+1  A: 

You would want to start with an IfExists style method, and then skip saving your changes unless you actually have changes.

As noted by Lucas, try-catch blocks have a large overhead if you fall into the catch block, so generally you don't want to rely on that unless there is no possible way of determining if the item already exists.

Don't use a try-catch to do an If statement's job. Try-catch is for unusual unanticipated events.

EDIT In your updated code, you are failing to catch an exception that would be thrown by the "AddToXXXXXX" method.

You should do

If(!XXXXXX.Contains(newItemValue))
{
   try
   {
      add...
      savechanges...
   }
   catch
   {

   }
}

alternatively, you could separate out the Add and Savechanges into different try-catch blocks, but that is only necessary if SaveChanges gets executed even when Add fails.