Generic repository with UoW and simple injector

Question

I'm implementing a generic repository + UoW pattern with EF6 and simple injector. I feel there is room for improvements. Please let me know if I left out some relevant code, I tried to keep it breif.

Do you see any need for the unit of work to keep track of the repositories (in its _repositories field)? As you can see I'm currently not using it.

Another important thing here is that I want one EF context, LGTContext, per web request. But as far as I understand, it would be the case due to the Lifestyle.Scoped registration lifestyle.

Any other thoughts?

Generic repository:

public abstract class DatabaseRepository<T> : IRepository<T> where T : BaseEntity
{
    protected readonly DbSet<T> DbSet;
    protected readonly LGTContext Context;

    protected DatabaseRepository(IUnitOfWork unitOfWork, LGTContext context)
    {
        Context = context;
        DbSet = Context.Set<T>();

        unitOfWork.Register(this);
    }

    public Task<PagedResult<T>> Find(Expression<Func<T, bool>> predicate, int pageNumber, int pageSize)
    {
        throw new NotImplementedException();
    }

    public virtual async Task<IEnumerable<T>> Find(Expression<Func<T, bool>> predicate)
    {
        return await DbSet.Where(predicate).ToListAsync();
    }

    public virtual async Task<IEnumerable<T>> GetAll()
    {
        return await DbSet.ToListAsync();
    }

    public virtual async Task<T> GetSingle(object id)
    {
        return await DbSet.FindAsync(id);
    }

    public virtual void Add(T entity)
    {
        DbSet.Add(entity);
    }

    public virtual void Delete(T entity)
    {
        if (Context.Entry(entity).State == EntityState.Detached)
        {
            DbSet.Attach(entity);
        }
        DbSet.Remove(entity);
    }

    public virtual void Edit(T entity)
    {
        DbSet.Attach(entity);
        Context.Entry(entity).State = EntityState.Modified;
    }
}

Unit of work:

public class UnitOfWork : IUnitOfWork, IDisposable
{  
    private readonly Dictionary<Type, IRepository> _repositories = new Dictionary<Type, IRepository>();
    private readonly LGTContext _context;
    private bool _disposed;

    public UnitOfWork(LGTContext context)
    {
        _context = context;
    }

    public async Task CommitAsync()
    {
        try
        {
            await _context.SaveChangesAsync();
        }
        catch (DbEntityValidationException e)
        {
            // todo: handle validation errors...
            throw;
        }
    }

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

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                _context.Dispose();
            }
        }
        _disposed = true;
    }

    public void Register(IRepository repository)
    {
        _repositories.Add(repository.GetType(), repository);
    }
}

Simple injector registration:

container.Register<IUnitOfWork, UnitOfWork>(Lifestyle.Scoped);
container.Register(typeof(IRepository<>), 
                        new[] { typeof(DatabaseRepository<>).Assembly }, Lifestyle.Scoped);
container.Register<LGTContext>(Lifestyle.Scoped);

Usage psuedo code:

public SomeController(IUnitOfWork uow, IRepository<SomeEntity> repo)
{
    var entities = await repo.GetAll();
    // modify some entities...
    await uow.CommitAsync();
}

Answer 1

Putting aside the fact that EF already provides both a UoW and Generic Repository, there's still a huge flaw with your UnitOfWork. It disposes a resource it doesn't own. You pass the context in as a constructor arg, store it as a member, and then code analysis yelled at you for not implementing IDisposable, so you disposed of it. The problem is, the UoW has no way of knowing that it's supposed to dispose of the context, because it was injected into the class.

In all the years I've been working with .Net, not once have I actually needed to implement IDisposable. Every single time that I thought I needed to, I didn't. I just needed to keep the lifetime of the IDisposable short and contained within a using block. That's possible if you abandon this folly and implement a domain specific service instead.

public class EFWidgetStore : IWidgetStore
{
    private readonly Func<DbContext> GetContext;
    public EFWidgetStore(Func<DbContext> contextFactory)
    {
        GetContext = contextFactory;
    }

    public Widget Get(int id)
    {
        using (context = GetContext())
        {
            //... get it
        }
    }

    public Widget Get(string name)
    {
         //...
    }

---

Next up, you've broken Liskov's substitution principle, which is common for these kinds of implementations.

public Task<PagedResult<T>> Find(Expression<Func<T, bool>> predicate, int pageNumber, int pageSize)
{
    throw new NotImplementedException();
}

Although, let's be honest. When will you ever treat a specific repository as an instance of the base class anyway... You've introduced a world of hurt on yourself for no gain what so ever. Please don't do this.