Try Catch in Repository

Question

None of the examples I have looked at for Repository Patterns include any kind of error handling. Why is this? Say for instance I have this:

public virtual TItem Insert<TItem>(TItem item) where TItem:class,new()
    {
        dbContext.Set<TItem>().Add(item);
        try
        {
            dbContext.SaveChanges();
        }
        catch (DbUpdateException)
        {

            return null;
        }

        return item;

    }

An instance where we violate a constraint. I catch the DbUpdateException... Where would this error handling live if not in the repository itself?

Goldstone

Answer 1

For the most part, a repository doesn't need to worry about handling exceptions. The classes that are consuming the repositories should handle that. In your example, why would want to return null if an insert error occurs? Isn't that less clear than just throwing an exception?

For example, let's say we want to insert a record through the repository and then print out the new ID. Assume that the insert is going to fail for any reason.

var myNewItem = myRepository.Insert(myItem);
Console.WriteLine("MyItem added with ID: {0}", myNewItem.ID);

Following the pattern in your question, you'd get a NullReference exception on the second line if the Insert fails. That's a little strange. It's more clear to see the DbUpdateException on the first line. It's also better to be able to count on Insert always either returning a valid instance or throwing an exception.

Mumps

I have no issue with having it throw the DbUpdate exception, it is cleaner than the NullRefrence I agreee. Back in the sotred procedure day we would use if not exists. Obviously I have constaints on the datamodel. So how do you make an entity smart enough to check for a record before doing an insert? – Goldstone

One approach would be to use some sort of Validator before you do the insert to check against the constraints so you can provide friendly error messages. Or you could catch the constraint exceptions thrown by the database in whatever is consuming the repository. Either way, I don't think it's the repository's job to figure out whether or not a record can be inserted. – Mumps

I agree, I will cross that bridge when I finish the repository code and actually wire up some DI for the consumer. – Goldstone

@rsbarro: when you use a validator to check constraints before the modification and your sql databse is not running in sequential mode you need to lock the records during the read. Otherwise race conditions can change the data in between the check and the modification, which means you still have to handle the sql exceptions. For that reasons it makes mostly more sense to use the database functionality instead of reprograming well established database features. – Samuelson

Answer 2

In a properly-designed system, the constraints should never be able to be violated. Make your entities smarter: don't use blind auto-implemented setters, for example.

The repository is not the place to do data validation. The proper place is:

• If you are checking simply "contractual" constraints, e.g. "quantity should be a nonnegative integer" or "don't pass me a null customer," put the logic in the entity itself (either setters or constructor or mutating methods, as appropriate).
• If you are checking business logic, put it in specialized objects (DDD specifications if you wish) that abstract away that logic.

The only time these exceptions should come up is when you run your unit integration tests and you get a failure, which will reveal that either your database constraints are mismatched with your entity, or your entity is implemented incorrectly. So you definitely shouldn't catch them.

England