C# - Entity Framework + Repository pattern + Unit of work pattern

Question

After hours spent on reading various articles about correct way to implement Repository patter & UoW patters I finally think that I have managed to create (hopefully good) implementation.

The worst thing is, not that it is too complicated, or anything like that, but it seems that there is no general agreement how it should be done properly. In one article you can find example, which is, according to article, the best way to do it, read another article, that same thing will be considered anti-pattern.

Guidelines / Desires

My personal guidelines / desires where that implementation must (as much as it is possible) :

1. Satisfy SOLID principles
2. Be testable
3. Be independent of framework
4. Be independent of DB

Inspirations

Among some others, I was heavily inspired by these implementations (I've also included few words as to why I didn't go for exactly same implementations) :

• Link 1
  • I disliked here that UoW has list of repositories, I think that UoW should't have knowledge of repositories
• Link 2
  • Same problem Link 1, as well as some other issues such as tight coupling with implementation of DbContext
• Link 3
  • Same problems as 1 & 2 (and has tight coupling with implementation of DbContext in repositories)
• Link 4
  • Awesome article and it seems that it has great solution, but at the moment, for current project, seems overly complicated

Motivation

I have been working on this project for around 2 years, but only now I am adding UoW & Repository patterns.

Reason why I haven't added them from the start was that, quiet sincerely, I couldn't understand why should I add Repo & UoW (and I dint't understand them as patterns, and bunch of conflicting arguments didn't help). Also at the beginning only persistence was done through EF and I liked "using pattern" with DbContext. So above model layer there was service layer which was using EF to access and manipulate data.

Over the time number of entities and services increased and some code for interaction with DB started duplicating. Also for various purposes RedisDb and Azure Document Db were introduced, so I am not only using EF to access/manipulate data. And API isn't only consumer of service (thus service layer), we also have ASP.net MVC, Azure Functions, etc.

I feel that our project is now mature enough to gradually refactor & introduce Repo & UoW patterns.

--- CODE ---

Here is current implementation, of course some parts of code were omitted for brevity.

Database Context Factory

/// <summary>
///     Interface for factory which is in charge of creating new DbContexts
/// </summary>
/// <autogeneratedoc />
public interface IDatabaseContextFactory
{
    /// <summary>
    /// Creates new Master Database Context.
    /// </summary>
    /// <returns>newly created MasterDatabaseContext</returns>
    /// <autogeneratedoc />
    DbContext MasterDbContext();
}


/// <inheritdoc />
/// <summary>
/// This is factory which is in charge of creating new DbContexts
/// It is implemented as Singleton as factory should be implemented (according to Gang of four) 
/// </summary>
/// <seealso cref="T:Master.Domain.DataAccessLayer.IDatabaseContextFactory" />
/// <autogeneratedoc />
public class DatabaseContextFactory : IDatabaseContextFactory
{
    /// <summary>
    /// This is implementation of singleton
    /// </summary>
    /// <remarks>
    /// To read more, visit: http://csharpindepth.com/Articles/General/Singleton.aspx (Jon skeet)
    /// </remarks>
    /// <autogeneratedoc />
    private static readonly DatabaseContextFactory instance = new DatabaseContextFactory();

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit (more about this: http://csharpindepth.com/Articles/General/Beforefieldinit.aspx)
    static DatabaseContextFactory()
    {

    }

    //so that class cannot be initiated 
    private DatabaseContextFactory()
    {
    }


    /// <summary>
    /// Instance of DatabaseContextFactory
    /// </summary>
    public static DatabaseContextFactory Instance => instance;

    /// <inheritdoc />
    /// <summary>
    /// Creates new MasterDatabaseContext
    /// </summary>
    /// <returns></returns>
    public DbContext MasterDbContext()
    {
        return new MasterDatabaseContext();
    }
}

Unit of Work

/// <inheritdoc />
/// <summary>
/// Unit of work interface
/// Maintains a list of objects affected by a business transaction and coordinates the writing out of changes and the resolution of concurrency problems.
/// </summary>
/// <seealso cref="T:System.IDisposable" />
/// <autogeneratedoc />
public interface IUnitOfWork : IDisposable
{
    /// <summary>
    /// Gets the database context. DatabaseContext is part of EF and itself is implementation of UoW (and repo) patterns
    /// </summary>
    /// <value>
    /// The database context.
    /// </value>
    /// <remarks> 
    /// If true  UoW was implemented this wouldn't be here, but we are exposing this for simplicity sake. 
    /// For example so that repository  could use benefits of DbContext and DbSet <see cref="DbSet"/>. One of those benefits are Find and FindAsnyc methods
    /// </remarks>
    /// <autogeneratedoc />
    DbContext DatabaseContext { get; }
    /// <summary>
    /// Commits the changes to database
    /// </summary>
    /// <returns></returns>
    /// <autogeneratedoc />
    void Commit();

    /// <summary>
    /// Asynchronously commits changes to database.
    /// </summary>
    /// <returns></returns>
    /// <autogeneratedoc />
    Task CommitAsync();

}


/// <inheritdoc />
/// <summary>
/// This is implementation of UoW pattern
/// </summary>
/// <remarks>
/// Martin Fowler: "Maintains a list of objects affected by a business transaction and coordinates the writing out of changes and the resolution of concurrency problems."
/// According to P of EEA, Unit of work should have following methods: commit(), registerNew((object), registerDirty(object), registerClean(object), registerDeleted(object)
/// The thing is DbContext is already implementation of UoW so there is no need to implement all this
/// In case that we were not using ORM all these methods would have been implemented
/// </remarks>
/// <seealso cref="T:Master.Domain.DataAccessLayer.UnitOfWork.IUnitOfWork" />
/// <autogeneratedoc />
public class UnitOfWork : IUnitOfWork
{
    /// <summary>
    /// Is instance already disposed
    /// </summary>
    /// <remarks>
    /// Default value of bool is false
    /// </remarks>
    /// <autogeneratedoc />
    private bool _disposed;

    /// <summary>
    /// Initializes a new instance of the <see cref="UnitOfWork"/> class.
    /// </summary>
    /// <param name="dbContextfactory">The database context factory.</param>
    /// <exception cref="ArgumentNullException">
    /// dbContextfactory
    /// or
    /// MasterDbContext - Master database context cannot be null
    /// </exception>
    /// <autogeneratedoc />
    public UnitOfWork(IDatabaseContextFactory dbContextfactory)
    {
        if (dbContextfactory == null)
        {
            throw new ArgumentNullException(nameof(dbContextfactory));
        }

        var MasterDbContext = dbContextfactory.MasterDbContext();

        if (MasterDbContext == null)
        {
            throw new ArgumentNullException(nameof(MasterDbContext), @"Master database context cannot be null");
        }

        DatabaseContext = MasterDbContext;
    }

    /// <summary>
    /// Gets the database context. DatabaseContext is part of EF and itself is implementation of UoW (and repo) patterns
    /// </summary>
    /// <value>
    /// The database context.
    /// </value>
    /// <remarks>
    /// If true  UoW was implemented this wouldn't be here, but we are exposing this for simplicity sake.
    /// For example so that repository  could use benefits of DbContext and DbSet <see cref="DbSet" />. One of those benefits are Find and FindAsnyc methods
    /// </remarks>
    /// <autogeneratedoc />
    public DbContext DatabaseContext { get; }

    /// <inheritdoc />
    /// <summary>
    /// Commits the changes to database
    /// </summary>
    /// <autogeneratedoc />
    public void Commit()
    {
         DatabaseContext.SaveChanges();
    }

    /// <inheritdoc />
    /// <summary>
    /// Asynchronously commits changes to database.
    /// </summary>
    /// <returns></returns>
    /// <autogeneratedoc />
    public async Task CommitAsync()
    {
        await DatabaseContext.SaveChangesAsync();
    }


    /// <inheritdoc />
    /// <summary>
    /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
    /// </summary>
    /// <autogeneratedoc />
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    /// <summary>
    /// Releases unmanaged and - optionally - managed resources.
    /// </summary>
    /// <param name="disposning"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
    /// <autogeneratedoc />
    protected virtual void Dispose(bool disposning)
    {
        if (_disposed)
            return;


        if (disposning)
        {
            DatabaseContext.Dispose();
        }


        _disposed = true;
    }

    /// <summary>
    /// Finalizes an instance of the <see cref="UnitOfWork"/> class.
    /// </summary>
    /// <autogeneratedoc />
    ~UnitOfWork()
    {
        Dispose(false);
    }

}

Generic Repository

/// <summary>
/// Generic repository pattern implementation
/// Repository  Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects.
/// </summary>
/// <remarks>
/// More info: https://martinfowler.com/eaaCatalog/repository.html
/// </remarks>
/// <typeparam name="TEntity">The type of the entity.</typeparam>
/// <typeparam name="TKey">The type of the key.</typeparam>
/// <autogeneratedoc />
public interface IMasterRepository<TEntity, in TKey> where TEntity : class
{
    /// <summary>
    ///     Gets entity (of type) from repository based on given ID
    /// </summary>
    /// <param name="id">The identifier.</param>
    /// <returns>Entity</returns>
    /// <autogeneratedoc />
    TEntity Get(TKey id);

    /// <summary>
    /// Asynchronously gets entity (of type) from repository based on given ID
    /// </summary>
    /// <param name="id">The identifier.</param>
    /// <returns></returns>
    /// <autogeneratedoc />
    Task<TEntity> GetAsnyc(TKey id);

    /// <summary>
    ///     Gets all entities of type from repository
    /// </summary>
    /// <returns></returns>
    /// <autogeneratedoc />
    IEnumerable<TEntity> GetAll();

    /// <summary>
    ///  Asynchronously gets all entities of type from repository
    /// </summary>
    /// <returns></returns>
    /// <autogeneratedoc />
    Task<IEnumerable<TEntity>> GetAllAsync();

    /// <summary>
    ///     Finds all entities of type which match given predicate
    /// </summary>
    /// <param name="predicate">The predicate.</param>
    /// <returns>Entities which satisfy conditions</returns>
    /// <autogeneratedoc />
    IEnumerable<TEntity> Find(Expression<Func<TEntity, bool>> predicate);
}


//Note to self: according to P of EAA Repo plays nicely with QueryObject, Data mapper and Metadata mapper - Learn about those !!!



/// <summary>
/// Generic repository pattern implementation
/// Repository  Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects.
/// </summary>
/// <typeparam name="TEntity">The type of the entity.</typeparam>
/// <typeparam name="TKey">The type of the key.</typeparam>
/// <seealso cref="Master.Domain.DataAccessLayer.Repository.Generic.IMasterRepository{TEntity, TKey}" />
/// <inheritdoc cref="IMasterRepository{TEntity,TKey}" />
public class MasterRepository<TEntity, TKey> : IMasterRepository<TEntity, TKey>
    where TEntity : class 
{

    /// <summary>
    /// DbSet is part of EF, it holds entities of the context in memory, per EF guidelines DbSet was used instead of IDbSet 
    /// </summary>
    /// <remarks>
    /// <para>
    /// Even though we are not 100% happy about this, 
    /// We decided to go with this instead of (for example) IEnumerable so that we can use benefits of <see cref="DbSet"/>
    /// Those benefits for example are Find and FindAsync methods which are much faster in fetching entities by their key than for example Single of First methods
    /// </para>
    /// </remarks>
    /// <autogeneratedoc />
    private readonly DbSet<TEntity> _dbSet;


    /// <summary>
    /// Initializes a new instance of the <see cref="MasterRepository{TEntity, TKey}"/> class.
    /// </summary>
    /// <param name="unitOfWork">The unit of work.</param>
    /// <exception cref="ArgumentNullException">unitOfWork - Unit of work cannot be null</exception>
    /// <autogeneratedoc />
    public MasterRepository(IUnitOfWork unitOfWork)
    {
        if (unitOfWork == null)
        {
            throw new ArgumentNullException(nameof(unitOfWork), @"Unit of work cannot be null");
        }

        _dbSet = unitOfWork.DatabaseContext.Set<TEntity>();
    }

    /// <inheritdoc />
    /// <summary>
    /// Gets entity with given key
    /// </summary>
    /// <param name="id">The key of the entity</param>
    /// <returns>Entity with key id</returns>
    public TEntity Get(TKey id)
    {
        return _dbSet.Find(id);
    }

    /// <inheritdoc />
    /// <summary>
    /// Asynchronously gets entity with given key
    /// </summary>
    /// <param name="id">The key of the entity</param>
    /// <returns>Entity with key id</returns>
    public async Task<TEntity> GetAsnyc(TKey id)
    {
         return await _dbSet.FindAsync(id);
    }

    /// <inheritdoc />
    /// <summary>
    /// Gets all entities
    /// </summary>
    /// <returns>List of entities of type TEntiy</returns>
    public IEnumerable<TEntity> GetAll()
    {
        return _dbSet.ToList();
    }

    public async Task<IEnumerable<TEntity>> GetAllAsync()
    {
        return await _dbSet.ToListAsync();

    }

    public IEnumerable<TEntity> Find(Expression<Func<TEntity, bool>> predicate)
    {
        return _dbSet.Where(predicate).ToList();
    }

}

Saved Movies Repository

 /// <inheritdoc />
/// <summary>
/// Repository for dealing with <see cref="T:Master.Domain.Model.MovieAggregate.SavedMovie" /> entity
/// </summary>
/// <seealso cref="!:Master.Domain.DataAccessLayer.Repository.Generic.IMasterRepository{Master.Domain.Model.MovieAggregate.SavedMovie,System.Guid}" />
/// <autogeneratedoc />
public interface ISavedMoviesRepository : IMasterRepository<SavedMovie, Guid>
{
    /// <summary>
    /// Asynchronously Gets number of saved Movies for the user
    /// </summary>
    /// <param name="user">The user.</param>
    /// <returns>Number of saved Movies</returns>
    /// <autogeneratedoc />
    Task<int> CountForUser(Model.UserAggregate.User user);
}


/// <inheritdoc cref="ISavedMoviesRepository" />
/// />
/// <summary>
///     Repository for dealing with <see cref="T:Master.Domain.Model.MovieAggregate.SavedMovie" /> entity
/// </summary>
/// <seealso cref="!:Master.Domain.DataAccessLayer.Repository.Generic.MasterRepository{Master.Domain.Model.MovieAggregate.SavedMovie, System.Guid}" />
/// <seealso cref="T:Master.Domain.DataAccessLayer.Repository.SavedMovies.ISavedMoviesRepository" />
/// <autogeneratedoc />
public class SavedMovieRepository : MasterRepository<SavedMovie, Guid>, ISavedMoviesRepository
{
    /// <summary>
    ///     Ef's DbSet - in-memory collection for dealing with entities
    /// </summary>
    /// <autogeneratedoc />
    private readonly DbSet<SavedMovie> _dbSet;

    /// <inheritdoc />
    /// <summary>
    ///     Initializes a new instance of the
    ///     <see cref="T:Master.Domain.DataAccessLayer.Repository.SavedMovies.SavedMovieRepository" /> class.
    /// </summary>
    /// <param name="unitOfWork">The unit of work.</param>
    /// <exception cref="T:System.ArgumentNullException"></exception>
    /// <autogeneratedoc />
    public SavedMovieRepository(UnitOfWork.UnitOfWork unitOfWork) : base(unitOfWork)
    {
        if (unitOfWork == null)
            throw new ArgumentNullException();
        _dbSet = unitOfWork.DatabaseContext.Set<SavedMovie>();
    }

    /// <inheritdoc />
    /// <summary>
    ///     Asynchronously Gets number of saved Movies for the user
    /// </summary>
    /// <param name="user">The user.</param>
    /// <returns>
    ///     Number of saved Movies
    /// </returns>
    /// <exception cref="T:System.ArgumentNullException">user - User cannot be null</exception>
    /// <autogeneratedoc />
    public async Task<int> CountForUser(Model.UserAggregate.User user)
    {
        if (user == null)
            throw new ArgumentNullException(nameof(user), @"User cannot be null");

        return await _dbSet.CountAsync(r => r.UserWhoSavedId == user.Id);
    }
}

Saved movies service

/// <inheritdoc />
/// <summary>
///     This is service for handling saved Movies!
/// </summary>
/// <seealso cref="T:Master.Infrastructure.Services.SavedMovieService.Interfaces.ISavedMovieService" />
/// <autogeneratedoc />
public class SavedMovieService : ISavedMovieService
{
    /// <summary>
    /// The saved Movies repository <see cref="ISavedMoviesRepository"/>
    /// </summary>
    /// <autogeneratedoc />
    private readonly ISavedMoviesRepository _savedMoviesRepository;

    /// <summary>
    /// The unit of work <see cref="IUnitOfWork"/>
    /// </summary>
    /// <autogeneratedoc />
    private readonly IUnitOfWork _unitOfWork;

    /// <summary>
    /// The user repository <see cref="IUserRepository"/>
    /// </summary>
    /// <autogeneratedoc />
    private readonly IUserRepository _userRepository;

    /// <summary>
    /// Initializes a new instance of the <see cref="SavedMovieService"/> class.
    /// </summary>
    /// <param name="savedMoviesRepository">The saved Movies repository.</param>
    /// <param name="userRepository">The user repository.</param>
    /// <param name="unitOfWork">The unit of work.</param>
    /// <autogeneratedoc />
    public SavedMovieService(ISavedMoviesRepository savedMoviesRepository, IUserRepository userRepository,
        IUnitOfWork unitOfWork)
    {
        _savedMoviesRepository = savedMoviesRepository;
        _userRepository = userRepository;
        _unitOfWork = unitOfWork;
    }

    public Task<int> CountNumberOfSavedMoviesForUser(string userId)
    {
        if (string.IsNullOrEmpty(userId))
            throw new ArgumentNullException(nameof(userId), @"User id must not be empty");


        var user = _userRepository.Get(userId);
        return _savedMoviesRepository.CountForUser(user);
    }

}

Web Api Controller

[Authorize]
[RoutePrefix("api/Saved")]
[ApiVersion("2.0")]
public class SavedController : ApiController
{
    private readonly ISavedMovieService _savedMovieService;


    /// <inheritdoc />
    /// <summary>
    ///     Initializes a new instance of the <see cref="T:Master.Infrastructure.Api.V2.Controllers.SavedController" /> class.
    /// </summary>
    /// <param name="savedMovieService">The saved Movie service.</param>
    /// <autogeneratedoc />
    public SavedController(ISavedMovieService savedMovieService)
    {        
        _savedMovieService = savedMovieService;
    }

    public async Task<IHttpActionResult> GetNumberOfSavedForUser()
    {
        var cnt = await _savedMovieService.CountNumberOfSavedMoviesForUser(User.Identity.GetUserId());

        return Ok(cnt);
    }
}

Ninject Configuration

(Only important parts)

        kernel.Bind<MasterDatabaseContext>().ToSelf().InRequestScope();
        kernel.Bind<IDatabaseContextFactory>().ToMethod(c => DatabaseContextFactory.Instance).InSingletonScope();
        kernel.Bind<IUnitOfWork>().To<UnitOfWork>().InRequestScope();

        kernel.Bind(typeof(IMasterRepository<,>)).To(typeof(MasterRepository<,>));

        kernel.Bind<ISavedMoviesRepository>().To<SavedMovieRepository>();
        kernel.Bind<IUserRepository>().To<UserRepository>();
        kernel.Bind<ISavedMovieService>().To<SavedMovieService>();

FINALLY

Most of the code is commented and has my remarks why I did it, the way I did it, but not everything is completely finished and properly commented as this is work in progress.

I hope to get comments whether this is good implementation and will it work well in production (regarding multithreading, scalability, consistency etc.), as well as your comments on code (is it easily understood, clean etc). Lastly, please point out any mistakes I have made.

Answer 1

I like the Factory. I called it Create() instead of MasterDbContext. :) After that, I'd like to talk about it a bit. I used to try to do things like this too since it felt like that is what I was reading all over the place. However, EF by itself already is a Unit of Work with Repositories (DbSet) built in. Thus, creating another one over the top doesn't add anything at best and takes away features at worst. If you are looking for something a little different, read up on CQS (Command Query Separation). With this style, you can go directly against the DbContext inside a command or query specifically and select back out what you need. Each command/query is specific to the business problem at hand. You could even swap EF out for Dapper if you wanted to for certain queries. If this doesn't really help you out at all, let me know and I can delete this answer.

Edit: One thing I would mention is that you create some sort of interface that represents a user. You can use IoC to link that to some class in the UI that pulls it from the User.Identity if you need to. That way, you aren't passing the UserId/UserName into the method declaration but instead are using IoC to get that information.

Answer 2

I understand how you don't like to have a reference to the repository interfaces in your uow. I could argue against it :), but I won't :)

I'm not sure about Ninject, I've been using Autofac for a while now and it has a feature which enables you to inject a list of services that implement the same interface into your other services:

http://docs.autofac.org/en/latest/resolve/relationships.html#enumeration-ienumerable-b-ilist-b-icollection-b

If Ninject has a similar option, maybe you could use this. Create one repository interface, have all your entity-specific repository interfaces extend this and then your concrete repositories can implement the extended interfaces. Then register the concrete repository types with the container and inside the uow, you can store them in a dictionary or whatever and later resolve from that.

This way, if you have to introduce a new repository, you just have to register it into the container, Autofac will inject it with this enumeration implicit relationship.