Repository pattern and collection error handling

Question

Say I've got the repository:

interface IRepo
{
    Foo Get(int id);
    void Add(Foo f);
}

Now, there is requirement that I can only have one Foo with given property, say...Id. Obviously, if I implement IRepo with some SQL backend and will map Foo.Id to Primary Key, I get something like this for free - I will get it probably as some kind of PKViolationException. But this is becoming implementation-specific, so wherever I use this IRepo implementation and start catching those exceptions, I'm losing benefits of loose coupling.

Now, how would I fix this? Should I add some kind of service layer that would first check whether the object exist, and then throw some exception that would be repository independent?

class Service
{
    IRepo repo;

    public void AddFoo(Foo foo)
    {
        if(repo.Get(foo.Id) != null)
            repo.Add(foo);
        else
            throw new FooAlreadyExistsException(foo.Id);
    }
}

This solution seems bad, because the repo.Add(foo) can still throw some exception, especially when object with given Id has been added right after the check (by some other activity), so I just added one additional call to my infrastructure for little or no benefits.

It seems like I should just be careful and implement the IRepo with such exception in mind (could catch PKViolationException and turn it into FooAlreadyExistsException in example SQL implementation), but what to do to be sure that every IRepo implementation follows such specification?

How do you tackle those issues generaly?

Answer 1

"It seems like I should just be careful and implement the IRepo with such exception in mind (could catch PKViolationException and turn it into FooAlreadyExistsException in example SQL implementation)"

You're right on the money with this. The exceptions being thrown become part of the interface contract, and implementations must adhere to this contract. The compiler will not enforce this for you so you have to be absolutely clear about the expectation.

"but what to do to be sure that every IRepo implementation follows such specification?"

As the interface writer you cannot be held responsible for the classes that implement it. If other classes are defective and expose leaky abstractions, that is their defect. All you can do is be absolutely clear about the contract you're defining.

The purpose of the repository is to abstract away the implementation details of the database, that includes its exceptions. So, you should also abstract away implementation specific exceptions...

interface IRepo
{
    /// <exception cref="FooAlreadyExistsException">Thrown if the specified Foo object already exists in the database.</exception>
    void Add(Foo f);
}

class ConcreteRepo
{
    public void Add(Foo f)
    {
        try
        {
            // Database stuff...
        }
        catch (PKViolationException e)
        {
            throw new FooAlreadyExistsException(e);
        }
    }
}

Answer 2

First of all, think about the following question: what's the probability that you change the underlying backing store used by your repository (i.e. go from a database with primary keys to some unknown datastore not having the notion of unique IDs)? I would guess that the probability is very close to 0. YAGNI applies here. Don't start implementing code in case some very unlikely unknown change occurs. When the change actually occurs, then do the necessary changes.

Then, regarding your example, why don't you generate a new ID (or let the underlying datastore do it for you) for the entity being added rather than forcing the client code to do it before? IDs should be autogenerated, using a database sequence, or a UUID, or n autoincrement column.