EF6 Code First unit of work pattern with IoC/DI

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 : where T : class
{
IQueryable AsQueryable();

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

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

public interface IUnitOfWork : IDisposable
{
IGenericRepository OrderRepository { get; }
IGenericRepository CustomerRepository { get; }
IGenericRepository EmployeeRepository { get; }

void Commit();
}

public class EfUnitOfWork : DbContext, IUnitOfWork
{
private readonly EfGenericRepository _OrderRepo;
private readonly EfGenericRepository _customerRepo;
private readonly EfGenericRepository _employeeRepo;

public DbSet Orders { get; set; }
public DbSet Customers { get; set; }
public DbSet Employees { get; set; }

public EfUnitOfWork()
{
_orderRepo = new EfGenericRepository(Orders);
_customerRepo = new EfGenericRepository(Customers);
_employeeRepo = new EfGenericRepository(Employees);
}

#region IUnitOfWork Implementation

public IGenericRepository OrderRepository
{
get { return _orderRepo; }
}

public IGenericRepository CustomerRepository
{
get { return _customerRepo; }
}

public IGenericRepository EmployeeRepository
{
get { return _employeeRepo; }
}

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

#endregion
}

public class EfGenericRepository : IGenericRepository
where T : class
{
private readonly DbSet_dbSet;

public EfGenericRepository(DbSet dbSet)
{
_dbSet = dbSet;
}

#region IGenericRepos

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
{
    //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 { }

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
{
    IEnumerable 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. 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 : where T : class
{
    IEnumerable 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.

StackExchange Code Review Q#63791, answer score: 4