8
\$\begingroup\$

I have been looking through Code Review on the subject of Unit of Work and Repository patterns and realised that my implementation appears to provide very similar functionality but in reverse class order/hierarchy. (Couldn't think of a better way to describe it sorry)

This is one of the questions I feel mine is similar to but in reverse.

I have added my code below. I would appreciate any feedback on my implementation you can offer regarding correctness, efficiency, and any suggestions.

UnitOfWork.cs - UnitOfWork + IUnitOfWork

public class UnitOfWork : IUnitOfWork
{
    PropertyInfoEntities _context = null;

    public IXXXXXRepository XXXXXRepository { get; set; }
    public IPersonRepository PersonRepository { get; set; }
    public IPersonLoginRepository PersonLoginRepository { get; set; }
    public IPropertyApplicationRepository PropertyApplicationRepository { get; set; }
    public ISaleTypeRepository SaleTypeRepository { get; set; }
    public IStatusRepository StatusRepository { get; set; }
    public ITownRepository TownRepository { get; set; }
    public ITypeRepository TypeRepository { get; set; }

    public UnitOfWork() : this(new PropertyInfoEntities()) { }

    public UnitOfWork(PropertyInfoEntities context)
    {
        _context = context;
        InitRepositories();
    }

    private void InitRepositories()
    {
        XXXXXRepository = new XXXXXRepository(_context);
        PersonRepository = new PersonRepository(_context);
        PersonLoginRepository = new PersonLoginRepository(_context);
        PropertyApplicationRepository = new PropertyApplicationRepository(_context);
        SaleTypeRepository = new SaleTypeRepository(_context);
        StatusRepository = new StatusRepository(_context);
        TownRepository = new TownRepository(_context);
        TypeRepository = new TypeRepository(_context);
    }

    public void Save()
    {
        _context.SaveChanges();
    }

    #region IDisposable Members

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing == true)
        {
            _context = null;
        }
    }

    ~UnitOfWork()
    {
        Dispose(false);
    }

    #endregion
}

public interface IUnitOfWork : IDisposable
{
    IXXXXXRepository XXXXXRepository { get; set; }
    IPersonRepository PersonRepository { get; set; }
    IPersonLoginRepository PersonLoginRepository { get; set; }
    IPropertyApplicationRepository PropertyApplicationRepository { get; set; }
    ISaleTypeRepository SaleTypeRepository { get; set; }
    IStatusRepository StatusRepository { get; set; }
    ITownRepository TownRepository { get; set; }
    ITypeRepository TypeRepository { get; set; }
    void Save();
}

XXXXXRepository.cs - XXXXXRepository + IXXXXXRepository

public class XXXXXRepository : IXXXXXRepository
{
    PropertyInfoEntities _context = null;

    public XXXXXRepository() : this(new PropertyInfoEntities()) { }

    public XXXXXRepository(PropertyInfoEntities context)
    {
        _context = context;
    }

    public IQueryable<XXXXX> All
    {
        get { return _context.XXXXXs; }
    }

    public IQueryable<XXXXX> AllIncluding(params Expression<Func<XXXXX, object>>[] includeProperties)
    {
        IQueryable<XXXXX> query = _context.XXXXXs;
        foreach (var includeProperty in includeProperties)
        {
            query = query.Include(includeProperty);
        }
        return query;
    }

    public XXXXX Find(int id)
    {
        return _context.XXXXXs.Find(id);
    }
    public XXXXX FindNT(int id)
    {
        return _context.XXXXXs.AsNoTracking().Single(f => f.ID == id);
    }

    public void InsertOrUpdate(XXXXX XXXXX)
    {
        if (XXXXX.ID == default(int))
        {
            // New entity
            _context.XXXXXs.Add(XXXXX);
        }
        else
        {
            // Existing entity
            _context.Entry(XXXXX).State = System.Data.Entity.EntityState.Modified;
        }
    }

    public void Delete(int id)
    {
        var XXXXX = _context.XXXXXs.Find(id);
        _context.XXXXXs.Remove(XXXXX);
    }

    public void Save()
    {
        _context.SaveChanges();
    }

    public void Dispose()
    {
        _context.Dispose();
    }
}

public interface IXXXXXRepository : IDisposable
{
    IQueryable<XXXXX> All { get; }
    IQueryable<XXXXX> AllIncluding(params Expression<Func<XXXXX, object>>[] includeProperties);
    XXXXX Find(int id);
    XXXXX FindNT(int id);
    void InsertOrUpdate(XXXXX XXXXX);
    void Delete(int id);
    void Save();
}

Providing my implementation was not incorrect, I was planning to change the UoW Repository Properties to self instantiating fields to increase efficiency, and load into a controller using a BaseController. However reading through some other Code Review questions has made my implementation feel wrong.

EDIT (4 years on): Looking back, my implementation was horrendous; mainly because of the InitRepositories() method that was called by the UoW constructor. This meant that for every instantiation of the UoW, it would instantiate each and every repository, regardless of whether they may be used or not. Aside from this the implementation isn't too bad, though it does work in reverse order to most others. If you want to hold the repositories explicitly within the UoW, you should implement them as (forgive my terminology) lazily self-instantiating properties, like here. Nowadays my preference is to use URF.NET and customise it as required.

\$\endgroup\$
0

1 Answer 1

12
\$\begingroup\$

I get what you mean by "reversed":

unit-of-work depends on repository

As opposed to:

repository depends on unit-of-work

Makes sense, at least to me - the way I see UoW/Repository pattern (everybody seems to have their own take at this one, eh?), Entity Framework's DbContext is a unit-of-work, and an IDbSet<TEntity> is a repository.

Hence, I tend to agree with having unit-of-work depend on repositories and not the opposite. When we inherit DbContext, we expose IDbSet<TEntity> properties, and this is exactly what you've got here.

Thing is, if DbContext is a unit-of-work, and IDbSet<TEntity> is a repository... then what need is there to wrap it with infrastructure code that only buys additional complexity?

You're not showing how your UoW implementation is used in your controllers, but if you're using it directly, then you're playing with IQueryable<T> and you're not really wrapping anything, EF and Linq-to-Entities is bleeding out of every usage you're making of every repository call, making the extra abstraction not-quite-an-abstraction.

I have yet to see a UoW+Repository implementation with EF that will show me real benefits over using the DbContext directly in the controllers (or, more appropriately, in a dedicated, testable service class).

Instead, I tend to just go like this:

public Interface IUnitOfWork
{
    IDbSet<TEntity> Set<TEntity>();
    void Save();
}

public class SomeContext : DbContext, IUnitOfWork
{
    public void Save() // base method returns an int that I don't want
    {
        base.SaveChanges(); // qualifier "base" is redundant, specified for readability
    }
}

Then, I can inject an IUnitOfWork and get an IDbSet<TEntity> for any entity type, and do everything EF has in store for me with that interface; the IUnitOfWork merely enables mocking, so I can set it up to return a mock IDbSet<Person> when Set<Person>() is called. Keep. It. Simple.

\$\endgroup\$
3
  • \$\begingroup\$ +1 Thank-you for your feedback! I shall try out your implementation. However I have noticed that ASP.Net appear to implement it similarly to me in their tutorial: asp.net/mvc/tutorials/getting-started-with-ef-5-using-mvc-4/… (3/4 down page: Creating the Unit of Work Class) \$\endgroup\$ Commented Apr 15, 2014 at 10:21
  • 2
    \$\begingroup\$ From a note frame in that link: "You can also build an abstraction layer into your database context class by using IDbSet interfaces there instead of DbSet types for your entity sets. The approach to implementing an abstraction layer shown in this tutorial is one option for you to consider, not a recommendation for all scenarios and environments." ;) \$\endgroup\$ Commented Apr 15, 2014 at 10:35
  • \$\begingroup\$ Thank-you for your help Mat, I shall investigate further :) \$\endgroup\$ Commented Apr 15, 2014 at 13:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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