5
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ As you apparently already read, the DbSet<T> is already a repository. \$\endgroup\$ – Magus Sep 24 '14 at 22:31
  • 1
    \$\begingroup\$ Make sure you take a look at Ben's answer here \$\endgroup\$ – Mathieu Guindon Sep 25 '14 at 0:52
  • \$\begingroup\$ @Magus, does that mean there should be no repositories in my code? Or just not the generic one? Mat's Mug, I read his answer, but can't make any code out of it. \$\endgroup\$ – Quoter Sep 25 '14 at 7:09
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Hi Ben, first of all, thank you very much for your review/answer! If I remove EfUnitOfWork, 'IUnitOfWork' and the generic repository (if I understood your last paragraph correct), what would my unit of work pattern look like? Right now it sounds to me like I just need to create objects out of repositories in my controllers. Doesn't this go against UoW pattern? Do you have a concrete sample you used in your projects or how you would set it up? \$\endgroup\$ – Quoter Sep 25 '14 at 9:57
  • \$\begingroup\$ "I just need to create objects out of repositories in my controllers" What do you mean by that exactly? \$\endgroup\$ – Ben Aaronson Sep 25 '14 at 10:26
  • \$\begingroup\$ The code I gave in my question is something i still need to implement. Right now I just have for example OrderRepository which I create an object of in, for example home controller: public ActionResult Index() { this._dbContext = new OrderRepository(); _dbContext.GetAll(); return View(); }. This is the current situation. Trying to refactore that to use UoW. So in my previous comment I asked if I have to remove UoW etc, whats left of UoW pattern? \$\endgroup\$ – Quoter Sep 25 '14 at 11:07
  • \$\begingroup\$ @Quoter Basically yes. EF already gives you unit of work, and there isn't really any advantage to adding another layer on top of it. \$\endgroup\$ – Ben Aaronson Sep 25 '14 at 11:11
  • 2
    \$\begingroup\$ @Quoter Yes if you need multiple repositories, inject multiple. The generic repository is just a base class. If you find your repositories have common methods, then you can pull them into the generic repository. If not, don't. Think of it exactly like any other base class, it's there for methods common to all of its subclasses. So my advice is start without a generic repository, if you find all your repositories share certain methods, create a generic repository for those methods \$\endgroup\$ – Ben Aaronson Sep 25 '14 at 11:30
3
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Hi, thanks for your comment. What do you suggest I should change? How should the classes look like? \$\endgroup\$ – Quoter Sep 25 '14 at 7:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.