Hosea146
Hosea146

Reputation: 7702

Entity Framework - Repository and Unit of Work - Am I doing this right?

My repository looks like this:

public class SqlRepository<T> : IRepository<T> where T : class
{
    private ExchangeSiteContext _dbContext;
    private DbSet<T> _dbSet;

    #region [Constructor]
    public SqlRepository(ExchangeSiteContext context)
    {
        _dbContext = context;
        _dbSet = context.Set<T>();
    }
    #endregion

    /// <summary>
    /// Gets the DbContext of the repository
    /// </summary>
    public ExchangeSiteContext DbContext
    {
        get
        {
            return this._dbContext;
        }
    }

    /// <summary>
    /// Get a list of entities
    /// </summary>
    /// <returns>List of type T entities</returns>
    public IQueryable<T> GetList()
    {
        return this._dbSet.AsQueryable();
    }

    /// <summary>
    /// Get a list of entities by a predicate
    /// </summary>
    /// <param name="predicate">The predicate</param>
    /// <returns>IQueryable of T</returns>
    public IQueryable<T> GetList(Expression<Func<T, bool>> predicate)
    {
        return this._dbSet.Where(predicate).AsQueryable();
    }

    ...
    ...
}

My unit of work looks like this:

public class SqlUnitOfWork : IUnitOfWork, IDisposable
{
    #region [Private Variables]
    private ExchangeSiteContext _dbContext;
    private BicycleSellerListingRepository _bicycleSellerListingRepository;
    private UserProfileRepository _userProfileRepository;
    #endregion

    #region [Constructor]
    public SqlUnitOfWork()
    {
        this._dbContext = new ExchangeSiteContext();
    }
    #endregion

    #region [Custom Repositories]
    public BicycleSellerListingRepository BicycleSellerListingRepository
    {
        get
        {
            if (this._bicycleSellerListingRepository == null)
                this._bicycleSellerListingRepository = new BicycleSellerListingRepository(this._dbContext);

            return this._bicycleSellerListingRepository;
        }
    }

    public UserProfileRepository UserProfileRepository
    {
        get
        {
            if (this._userProfileRepository == null)
                this._userProfileRepository = new UserProfileRepository(this._dbContext);

            return this._userProfileRepository;
        }
    }
    #endregion

    ///
    /// Generic repository
    ///
    public SqlRepository<T> GenericRepository<T>() where T : class
    {
        return new SqlRepository<T>(this._dbContext);
    }

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

    public void Dispose()
    {
        this._dbContext.Dispose();
        this._dbContext = null;
    }
}

My repository is all generic. My unit of work has some custom repositories, mostly for cases where I can't perform a generic operation.

My question is, does this look right? I've never created a repository or unit of work before. This seems to work pretty well, but I'm not sure if I'm overlooking something.

Upvotes: 2

Views: 1772

Answers (2)

Ibrahim Najjar
Ibrahim Najjar

Reputation: 19423

The implementation of the Unit of Work and Repository patterns is still the subject of a huge debate in the developers world, nevertheless after reading so much about this i can give you a few guidelines i have gathered:

  • Don't overuse generics, if you really need a generic repository then only put methods in there that are truly shared between all of them and if you can avoid this all together i would it would be better.
  • Never put a method that filter based on Expression trees.
  • I can give you other thoughts but i would like to direct you to an article i wrote about this [not because its mine, i really did hard work on that], if you want you can check it out here.

Upvotes: 1

Sergey Berezovskiy
Sergey Berezovskiy

Reputation: 236218

There is no single correct implementation of repository and UoW (as for me, I prefer one where UoW is simple wrapper over DbContext, which is passed to repositories). But here is some issues I see in your implementation:

  • GetList methods are confusing. They are returning IQueryable instead of list. I think GetAll is more appropriate name.
  • You don't need to call _dbSet.AsQueryable(), because DbSet<T> implements IQueryable<T>. Simply return _dbSet.
  • Usually some method for including related entities for eager loading is created in generic repository.
  • If your repository is generic, then why are you using specific context? Use DbContext instead.
  • Why are you exposing DbContext from repository?
  • You are creating context in UoW. That makes dependency injection not possible.
  • Setting _dbContext to null is not necessary after you disposed it.

Upvotes: 3

Related Questions