John B
John B

Reputation: 20380

Is it ok to rely on a try-catch in a CreateOrUpdate method for the Entity Framework?

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

Upvotes: 2

Views: 607

Answers (3)

DevinB
DevinB

Reputation: 8316

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.

Upvotes: 1

Kredns
Kredns

Reputation: 37211

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.

Upvotes: 0

JaredPar
JaredPar

Reputation: 754803

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.

Upvotes: 7

Related Questions