Interface for unit of work pattern and repository pattern

Question

I'm trying to design a well defined yet simple interface for the unit of work and repository patterns. My UoW's are exposed to services and services then "get repositories" that it needs to query. I know returning IQueryable<T> for repositories is a religious war. Because repositories are only exposed to the service, all queries are performed inside the service and therefore I can test the queries. Is there anything I should change for these interfaces? All criticisms are greatly appreciated!

public interface IUnitOfWork : IDisposable
{
    bool IsActive { get; }

    bool WasCommitted { get; }

    /// <summary>
    /// Commits all changes made on the unit of work.
    /// </summary>
    void Commit();

    bool WasRolledBack { get; }

    /// <summary>
    /// Rolls back all changes made on the unit of work.
    /// </summary>
    void Rollback();

    /// <summary>
    /// Returns an instance of an entity with the specified key that is attached to the unit of work without
    /// loading the entity from a repository.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    T Load<T>(int id)
        where T : class;

    void Attach<T>(T entity)
        where T : class, IIdentifiable;

    void Detach<T>(T entity)
        where T : class;

    IRepository<T> GetRepository<T>()
        where T : class;
}

public interface IRepository<T>
    where T : class
{

    IUnitOfWork UnitOfWork { get; }

    void Add(T entity);

    void Remove(T entity);

    /// <summary>
    /// Returns an instance of an entity with the specified key that is attached to the unit of work by loading
    /// the entity from the repository.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    T Get(int id);

    IQueryable<T> All();
}

Answer 1

I think your interfaces are mostly well-defined, with a couple exceptions:

I don't think Load, Attach and Detach should be members of IUnitOfWork, but rather IRepository. It seems that the repository that manages objects of type T would be the best place to place methods that acts on that type.

The UnitOfWork property on IRepository does not belong there. Things that modify one repository shouldn't be allowed to go and get other repositories to modify on their own. If they need them, pass them in explicitly. Otherwise, you're hiding from potential callers the fact that your implementation actually depends on more than just the one repository, and hiding your dependencies like you're ashamed of them is one of my least favorite code smells.

Answer 2

Here is how I would shape it:

public interface IEntity
{
    GUID Id { get; }
}

public interface IUnitOfWork<TEntity> where TEntity : IEntity, class 
    : IDisposable
{
    void Commit();
    void Discard();
    void Track(TEntity entity);
    void Delete(TEntity entity);
}

public interface IRepository<TEntity> where TEntity : IEntity, class 
    : IDisposable
{
    IUnitOfWork<TEntity> UnitOfWork { get; }

    TEntity CreateNew();
    T FindOne(ISpecification criteria);
    IEnumerable<T> FindMandy(ISpecification criteria);
    IEnumerable<T> FetchAll();
}

public interface ISpecification
{
    // encapsulates everything what is needed for a query 
    // this might be an SQL-statement, LINQ-functor... 

    string CommandText { get; }
    object[] Arguments { get; set; }
    bool IsSatisfiedBy(object candidate);
}

Repository itself is read only, UnitOfWork is writeonly. I can't see how this design breaks Law of Demeter.

Answer 3

Your interface breaks the Law of Demeter. Or said simply I cannot use IUnitOfWork UnitOfWork { get; } on IRespository without understanding IUnitOfWork.

I would change IRespository too

public interface IRepository<T>
    where T : class
{

    bool IsActive { get; }

    bool WasCommitted { get; }

    /// <summary>
    /// Commits all changes made on the unit of work.
    /// </summary>
    void Commit();

    bool WasRolledBack { get; }

    /// <summary>
    /// Rolls back all changes made on the unit of work.
    /// </summary>
    void Rollback();

    /// <summary>
    /// Returns an instance of an entity with the specified key that is attached to the unit of work without
    /// loading the entity from a repository.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    T Load<T>(int id)
        where T : class;

    void Attach<T>(T entity)
        where T : class, IIdentifiable;

    void Detach<T>(T entity)
        where T : class;

    IRepository<T> GetRepository<T>()
        where T : class;

    void Add(T entity);

    void Remove(T entity);

    /// <summary>
    /// Returns an instance of an entity with the specified key that is attached to the unit of work by loading
    /// the entity from the repository.
    /// </summary>
    /// <param name="id"></param>
    /// <returns></returns>
    T Get(int id);

    IQueryable<T> All();
}

Notice how it is a copy of IUnitOfWork, is UnitOfWork really needed.

How did you create the interface?

This is where you should do TDD. Write the test first about what you want the code to do. Then your interface should be created in order to accomplish the test.

For example if you call the method Rollback(); and cannot write a test were WasRollbacked property was used, they you might not need the property.

Do not put in more than needed to any interface. They interface becomes noise. I would recommend reading Clean Code by Robert Martin to understand more of these ideas in a in depth way.

Answer 4

I don't remember exactly where I got my implementation, but a lot of it is borrowed from the Spring Framework...

The only three methods that my IUnitOfWork interface exposes are Initialize(), Commit(), and Rollback(). Then I inherit from that for an INhibernateUnitOfWork, which simply exposes the NHibernate Session via a property as well... I guess this part may depend on what you are using for the persistence storage mechanism...

But what it allows me to do in my repository class then is to simply take in the unit of work as an injectable dependency. Essentially, each of my Get(id), Load(id), Find(), Save(), Update(), etc. methods just make a call against the Repository.UnitOfWork.Session property.

In essence, it's really close to what you've already implemented... Not sure why you want the GetRepository method, but you may have some use case for that which I do not. For me, my UnitOfWork is Request lifetime object whose Initialize() and Commit() / Rollback() functionality is handled by an HTTP module hooking the BeginRequest and EndRequest events. That is, the UnitOfWork instance is only ever touched by said HTTP Module, and by the repositories that simply use it to get a reference to the current Session object.