views:

70

answers:

4

I have two tables, A and B and a simple table, X, that maintains a relationship between the two. X contains AID and BID as its primary key.

I'm using Linq-to-Sql to insert a relationship like:

public void InsertRelationship(int y, int z) {
  DataContext.X.InsertOnSubmit(new x { AID = y; BID = z });
}

The problem is that two calls to InsertRelationship() can be invoked in extreme circumstances so an exception will be thrown due to the duplicate record. This doesn't matter to me as I know the relationship then exists so I'm ignoring the exception.

Is it okay to ignore an exception in this case or is it still bad practice? Should I check that the relationship doesn't already exist before the insert? How would this affect performance?

Update

The duplicate call to InsertRelationship() cannot be avoided. It is a web application so I cannot stop the user opening two separate windows and invoking the method twice for example. The method would not be invoked twice through typical user interaction but I'm programming against the extreme case here. The percentage of duplicates will probably be very low but I can't be sure of exact numbers yet.

+1  A: 

Generally, you should check first.

But probably more accurately, is that you should construct the interface to your application such that it doesn't present the case where it is even possible, through typical user interaction, to even generate the case that you'd need to check.

Regardless of doing that, better practice is to check.

Noon Silk
As this is a web application I cannot avoid the possibility that user could invoke the method twice. I've updated the question to show this.
David G
As I said, regardless, it's better to check and return a nice error to them.
Noon Silk
+1  A: 

The main problem with just ignoring the exception is that an exception may be thrown for other problems not related at all with a duplicate record which you don't want to hide, like SQL timeouts or deadlocks among others.

The best solution would be to construct your application so the dupes insertion does not happen, the next best would be to check for their existence before doing the insertion.

If performance is critical, the dupes insertion cannot be avoided, the ratio of dupes insertion to working insertion is very small and you can tell that the raised exception is only due to the duplicate problem, you might be better off just catching the exception and ignoring it. But that's an if with a lot of conditions :-)

Vinko Vrsalovic
A: 

If you are sure that the record is not inserted only because it already exists the it should be okay. Other wise ignoring exception may hide something else dangerous.

TheVillageIdiot
+1  A: 

Throwing exceptions is expensive, in terms of performance. Also, the exception you get is never guaranteed to be for the same reason every time. You should always check if the row exists before adding it again, as you know that the problem could exist to begin with.

Exceptions are for exceptional circumstances - this isn't exceptional, this is part of the normal flow for creating data.

Sam
"Throwing exceptions is expensive", is it more expensive than checking that the record doesn't exist every time? The number of duplicate calls is probably less than 2%.
David G
I wouldn't worry about performance until you have hard evidence that checking the database first is actually causing problems in that area - until then, anything you do on that front is premature optimization. Checking first is the best pattern to follow because the intent of the code is clear, and you aren't hiding information by ignoring an exception.
Sam