Web API using Repository / UnitOfWork

Question

I'm looking for feedback on a repository I set up based on a blog post I read here.

I have just recently got more into .NET within the past year, so any feedback and/or best practices that can be thrown my way would be appreciated.

The branch I am working in can be seen here if interested.

IGenericRepository:

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 FirstOrDefault(Expression<Func<T, bool>> predicate);
    T GetById(int id);

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

IScrapedRepository:

public interface IScrapedRepository<T> : IGenericRepository<T> where T : class
{
    bool IsRefreshRequired();
    void RefreshData();
}

IUnitOfWork:

public interface IUnitOfWork : IDisposable
{
    IGenericRepository<OpeningOption> OpeningOptionRepository { get; }
    IGenericRepository<Location> LocationRepository { get; }

    void Commit();
}

EFGenericRepository:

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

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

    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);
    }

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

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

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

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

    public T GetById(int id)
    {
        return _dbSet.Find(id);
    }

    public void Add(T entity)
    {
        _dbSet.Add(entity);
    }

    public void Delete(T entity)
    {
        _dbSet.Remove(entity);
    }

    public void Attach(T entity)
    {
        _dbSet.Attach(entity);
    }
}

ScrapedRepository:

public class ScrapedRepository<T> : EfGenericRepository<T>, IScrapedRepository<T> where T : class 
{
    public ScrapedRepository(IDbSet<T> dbSet) : base(dbSet) 
    {
        if (this.IsRefreshRequired()) 
        {
            this.RefreshData();
        }
    }

    public virtual bool IsRefreshRequired()
    {
        return true;
    }

    public virtual void RefreshData()
    {
    }
}

LocationRepository:

public class LocationRepository : ScrapedRepository<Location>
{
    public LocationRepository(IDbSet<Location> dbSet) : base(dbSet) { } 

    public override bool IsRefreshRequired()
    {
        //check site for new data
    }

    public override void RefreshData()
    {
        //scrape data and store it
    }
}

EfUnitOfWork:

public class EfUnitOfWork : DbContext, IUnitOfWork
{
    private readonly OpeningOptionRepository _openingOptionRepo;
    private readonly LocationRepository _locationRepo;

    public DbSet<Entities.OpeningOption> OpeningOptions { get; set; }
    public DbSet<Entities.Location> Locations { get; set; }

    public EfUnitOfWork()
    {
        _openingOptionRepo = new OpeningOptionRepository(OpeningOptions);
        _locationRepo = new LocationRepository(Locations);
    }

    public IGenericRepository<Entities.OpeningOption> OpeningOptionRepository
    {
        get { return _openingOptionRepo; }
    }

    public IGenericRepository<Entities.Location> LocationRepository
    {
        get { return _locationRepo; }
    }

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

From there, I register IUnitOfWork with Simple Injector:

public class RepositorySetup
{
    public static void ConfigureInjections(Container container) 
    {
        container.Register<IDbContext, DaycareSearchEntities>(Lifestyle.Transient);
        container.Register<IUnitOfWork, EfUnitOfWork>(Lifestyle.Transient);
    }
}

I then use a BaseController to gain access to EFUnitOfWork in all my controllers:

public abstract class BaseController : ApiController
{
    protected IUnitOfWork UnitOfWork { get; set; }

    protected BaseController(IUnitOfWork unitOfWork)
    {
        this.UnitOfWork = unitOfWork;
    }

    protected IHttpActionResult Result<T>(T entity){
        return entity == null
            ? (IHttpActionResult)NotFound()
            : (IHttpActionResult)Ok<T>(entity)
        ;
    }
}

Then here is how I access my data in the controller:

public class OpeningOptionController : BaseController
{
    public OpeningOptionController(IUnitOfWork unitOfWork) : base(unitOfWork) { }

    [HttpGet]
    public IEnumerable<OpeningOption> GetAllOpeningOptions()
    {
        return UnitOfWork.OpeningOptionRepository.GetAll();
    }

    [HttpGet]
    public IHttpActionResult GetOpeningOption(int id)
    {
        var openingOption = UnitOfWork.OpeningOptionRepository.FirstOrDefault(o => o.ID == id);
        return Result<OpeningOption> (openingOption);
    }
}

Answer 1

Overall this looks quite good.

1. I would rename IGenericRepository to IRepository, I don't think Generic adds readability.
2. I don't like exposing IQueryable in a public interface, it is a leaky abstraction. You don't know when database server will execute the sql.
3. I would not have my UnitOfWork own repositories because it is difficult to test/mock.

Answer 2

The patterns here seem well refined. The only thing I would change is, depending on the size and complexity of the application, is to refactor the IUnitOfWork/Repo interaction out of the controller and into a service layer.

I'm personally working on a way to eliminate the IUnitOfWork out all together using some kind of factory.

Edit:

Here is an example snippet from my attached question. This is what I'd want to replace the IUnitOfWork with.

namespace EFDataAccess
{
    class RepositoryFactory
    {

        private readonly DbContext _context;

        public RepositoryFactory(DbContext context)
        {
            _context = context;
        } 

        public IGenericRepository<T> Get<T>() where T : Type
        {
            return new EFGenericRepository<T>(_context.Set<T>());
        }
    }
}