EF6 Code First unit of work pattern with IoC/DI

Question

I'm trying to implement the unit of work pattern with dependency injection / inversion of control and entity framework version 6.1.1 Code First, in an asp.net-mvc project.

public interface IGenericRepository<T> : where T : class
{
    IQueryable<T> AsQueryable();

    IEnumerable<T> GetAll();
    IEnumerable<T> Find(Expression<Func<T, bool>> predicate);
    T Single(Expression<Func<T, bool>>  predicate);
    T SingleOrDefault(Expression<Func<T, bool>> predicate);
    T First(Expression<Func<T, bool>> predicate);
    T GetById(int id);

    void Add(T entity);
    void Delete(T entity);
    void Attach(T entity);
}

public interface IUnitOfWork : IDisposable
{
    IGenericRepository<Order> OrderRepository { get; }
    IGenericRepository<Customer> CustomerRepository { get; }
    IGenericRepository<Employee> EmployeeRepository { get; }

    void Commit();
}

public class EfUnitOfWork : DbContext, IUnitOfWork
{
    private readonly EfGenericRepository<Order> _OrderRepo;
    private readonly EfGenericRepository<Customer> _customerRepo;
    private readonly EfGenericRepository<Employee> _employeeRepo;

    public DbSet<Order> Orders { get; set; }
    public DbSet<Customer> Customers { get; set; }
    public DbSet<Employee> Employees { get; set; }

    public EfUnitOfWork()
    {
        _orderRepo = new EfGenericRepository<Order>(Orders);
        _customerRepo = new EfGenericRepository<Customer>(Customers);
        _employeeRepo = new EfGenericRepository<Employee>(Employees);
    }

    #region IUnitOfWork Implementation

    public IGenericRepository<Order> OrderRepository
    {
        get { return _orderRepo; }
    }

    public IGenericRepository<Customer> CustomerRepository
    {
        get { return _customerRepo; }
    }

    public IGenericRepository<Employee> EmployeeRepository
    {
        get { return _employeeRepo; }
    }

    public void Commit()
    {
        this.SaveChanges();
    }

    #endregion
}

public class EfGenericRepository<T> : IGenericRepository<T>
                                      where T : class
{
    private readonly DbSet<T>_dbSet;

    public EfGenericRepository(DbSet<T> dbSet)
    {
        _dbSet = dbSet;
    }

    #region IGenericRepository<T> implementation

    public virtual IQueryable<T> AsQueryable()
    {
        return _dbSet.AsQueryable();
    }

    public IEnumerable<T> GetAll()
    {
        return _dbSet;
    }

    public IEnumerable<T> Find(Expression<Func<T, bool>> predicate)
    {
        return _dbSet.Where(predicate);
    }

    // And so on ...

    #endregion
}

And this is how I'm using it in a controller:

public HomeController(IUnitOfWork unitOfWork) { ..... }

But I've read that EF6 already has Unit of Work out of the box. How would that fit in this picture?

Also, with which dependency injector should I go with?

Answer 1

Unit of work

There are some issues of mechanics here. Normally with a generic repository, you want to be able to extend it, so you'd write:

public interface IOrderRepository : IGenericRepository<Order>
{
    //Some Order-specific queries here
}

But this would mean updating your IUnitOfWork every time too.

You're also having to add lots of annoying boilerplate code to your EfUnitOfWork to make it conform to the interface. The question here has a much nicer way of achieving the same thing.

However, my actual suggestion would be to remove IUnitOfWork altogeter, and simply add a Save (or, if you prefer, Commit) method to your repositories. Then any code which needs data access should be passed the repositories it needs directly, rather than ever being passed the DbContext.

The Generic Repository

I think the first hint that you're putting the cart before the horse here is the name you picked: IGenericRepository. The fact that it's generic absolutely does not need to be in the name. For one thing, it's already implied by the generic parameter. But more importantly, the fact that you're using the generic repository pattern, as opposed to just the repository pattern, is an unimportant detail.

The generic repository is simply a base class for your repositories. The only reason it's there is that there will be some methods you'll want on all of your actual repositories, and you don't want to have to repeat them.

Mat's Mug already gave the reasons in his answer not to expose IQueryable or take Expression or Func arguments in your repository, and I'd strongly echo those.

How to write your repositories

So given that, here's what I think a generic repository should look like to start out:

public interface IRepository<T> { }

Why is it empty? Because you don't know what methods you're going to need yet.

As soon as you need data access in one of your classes, write a repository for it:

public interface IOrderRepository : IRepository<T>
{
    IEnumerable<Order> GetAllPendingOrders(DateTime endDate);

    void Add(Order order);
}

(GetAllPendingOrders and Add being made up example data access methods that you might find yourself needing to use in some service class)

Note how these methods are defined by what the repository's consumer needs, they're not just aimed to exactly match what IDbSet already gives you.

After you have a few repositories, you'll find you have quite a bit of repetition. For example, Add would probably be on most or all of them. (GetAllPending... would not!) So then, you simply refactor by removing those methods and creating a generic version on the Repository<T>. This is just plain old vanilla extracting of a base class that you see all over the place, there's nothing magic about it just because it's relating to repositories.

Guessing

If you really don't like the idea of starting with an empty generic repository, there are some methods which using educated guesswork, you can be almost sure you'll need. So a starting point for your generic repository might be the following cut-down version of the one you posted:

public interface IGenericRepository<T> : where T : class
{
    IEnumerable<T> GetAll();
    T GetById(int id);
    void Add(T entity);
    void Save();
}

But to emphasise: don't just copy this without understanding why! Generic repositories are horribly badly explained in dozens of articles and blog posts across the internet, and it's really worth clearing up in your mind what they're actually for before using them.

Answer 2

Quick observation: You're exposing IQueryable, both directly and indirectly.

By not materializing the query results before returning from your repository methods, the client code might not be expecting its LINQ code to operate against LINQ-to-Entities..

Even though you're returning IEnumerable, deferred execution will cause the query to hit the database when the client code iterates the result, and this can mean unexpected exceptions, because the client code has all rights to assume it's working against LINQ-to-Objects and shouldn't have to worry about whether or not its actions can or cannot be correctly translated into T-SQL by the LINQ provider.