Unit of Work and Repository with Entity Framework 6

Question

Based on the reply of this question I have created the following code. I need to check whether it's good or not.

Here is my entity class:

public class Employee
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Designation { get; set; }
}

This is my db context implementation:

 public class MyDataContext<T> : DbContext where T:class
{
    private IDbSet<T> _dbSet;

    public MyDataContext() : base("name=DefaultConnectionString")
    {
        _dbSet = this.Set<T>();
    }

    public MyDataContext(IDbSet<T> dbSet )
        : base("name=DefaultConnectionString")
    {
        this._dbSet = dbSet;
    }

    public IDbSet<T> DbSetOjbect
    {
        get { return _dbSet; }
    }
}

Now I have implemented the EmployeeService business logic class and the IEmployee service class:

 public interface IEmployeeService
{
    List<Employee> GetEmployees();
}

Here is the implementation:

public class EmployeeService : IEmployeeService 
{
    private IDbSet<Employee> employee;

    public EmployeeService()
    {
        var employeeContext = new MyDataContext<Employee>();
        employee = employeeContext.DbSetOjbect;
    }

    public EmployeeService(IDbSet<Employee> employee) 
    {
        this.employee = employee;
    }

    public List<Employee> GetEmployees() 
    {
        return employee.ToList();
    }
}

The following is my controller code in ASP.NET MVC controller.

 public class EmployeeController : Controller
{
    private readonly IEmployeeService _employeeService;

    public EmployeeController()
    {
        _employeeService = new EmployeeService();
    }

    public EmployeeController(IEmployeeService employeeService)
    {
        _employeeService = employeeService;
    }

    public ActionResult Index()
    {
        return View(_employeeService.GetEmployees());
    }
}

I want to check whether it is a good approach for TDD Test Driven Development or not.

Answer 1

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IEnumerable<Employee> GetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

Answer 2

The Context in this situation isn't correct. The Context should have all your dbSets. With the UnitOfWork pattern there is only 1 instance of the Context. It is used by your repositories (DbSets) and by the UnitOfWork. Since there is only a single instance it allows you to call many services, each of which update your context, before calling UnitOfWork.Commit(). When you call UnitOfWork.Commit() all the changes you've made will get submitted together. In your above implementation you are creating a new Context in the EmployeeService which means in another service you will end up creating another instance of the Context and that is incorrect. The idea behind the UnitOfWork pattern is that you can chain together services before committing and the data gets saved as a single UnitOfWork.

Here's my context from a recent project, reduced in size. My IDataContext has some additional definitions for what I need to use from DbContext like:

public interface IDataContext : IDisposable
    {
        DbChangeTracker ChangeTracker { get; }
        int SaveChanges();
        DbEntityEntry<TEntity> Entry<TEntity>(TEntity entity) where TEntity : class;
        DbSet<TEntity> Set<TEntity>() where TEntity : class;
        DbSet Set(Type entityType);
        int> SaveChanges();
        public IDbSet<Function> Functions { get; set; }
        public IDbSet<PlaceHolder> PlaceHolders { get; set; }
        public IDbSet<Configuration> Configurations { get; set; }
        public IDbSet<Client> Clients { get; set; }
        public IDbSet<ParentClient> ParentClients { get; set; }
}

    public class DataContext : DbContext, IDataContext
        {
            public DataContext()
            {
                Configurations = Set<Configuration>();
                Clients = Set<Client>();
                ParentClients = Set<ParentClient>();
            }

            public IDbSet<Function> Functions { get; set; }
            public IDbSet<PlaceHolder> PlaceHolders { get; set; }
            public IDbSet<Configuration> Configurations { get; set; }
            public IDbSet<Client> Clients { get; set; }
            public IDbSet<ParentClient> ParentClients { get; set; }

            protected override void OnModelCreating(DbModelBuilder modelBuilder)
            {
                base.OnModelCreating(modelBuilder);
                new UserConfiguration().AddConfiguration(modelBuilder.Configurations);
                new ParentClientConfiguration().AddConfiguration(modelBuilder.Configurations);
                new ClientConfiguration().AddConfiguration(modelBuilder.Configurations);
                new EmailConfiguration().AddConfiguration(modelBuilder.Configurations);
                Configuration.LazyLoadingEnabled = false;
            }
        }

Here's my UnitOfWork. There are different ways of doing this. In some cases people expose all the repositories (DbSets) here, and then inject the UnitOfWork into their classes and extract whatever repositories they need. Personally I don't like having something in my services that exposes the entire data store so I follow the hiding approach. As you can see the approach I'm following only has a Commit(). Since the UnitOfWork and all the repositories (DbSets) share the same single instance of the Context they are all acting on the same data.

public interface IUnitOfWork : IDisposable
    {
        ICollection<ValidationResult> Commit();
    }

    public class UnitOfWork : IUnitOfWork
    {
        private readonly IDataContext _context;

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

        public ICollection<ValidationResult> Commit()
        {
            var validationResults = new List<ValidationResult>();

            try
            {
                _context.SaveChanges();
            }
            catch (DbEntityValidationException dbe)
            {
                foreach (DbEntityValidationResult validation in dbe.EntityValidationErrors)
                {
                    IEnumerable<ValidationResult> validations = validation.ValidationErrors.Select(
                        error => new ValidationResult(
                                     error.ErrorMessage,
                                     new[]
                                         {
                                             error.PropertyName
                                         }));

                    validationResults.AddRange(validations);

                    return validationResults;
                }
            }
            return validationResults;
        }

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

This brings us to your service classes. The catch here is that if you choose to inject IDbSets then you have to extract them from the context and inject them because you can't create them directly. Using Unity as the IOC (I recommend Autofac but had to use Unity for this project) it looks like this:

var context = container.Resolve<IDataContext>();
container.RegisterInstance(context.Functions, manager(typeof (ContainerControlledLifetimeManager)));
container.RegisterInstance(context.AuditRounds, manager(typeof (ContainerControlledLifetimeManager)));
container.RegisterInstance(context.Clients, manager(typeof (ContainerControlledLifetimeManager)));

In order to support this kind of code you'll need to something like what is above:

public EmployeeService(IDbSet<Employee> employee) 
    {
        this.employee = employee;
    }

Per your original concern you were not wanting to do something for "every" entity. Personally the effort is so small I don't concern myself with it but if that's important to you then there is the GenericRepository approach. In that approach we inject that single instance of the IContext and the repository extracts the IDbSet from the context using some EF functionality and you have a class like:

public GenericRepository<T> : IGenericRespository<T>
{
    private SchoolContext _context;

    public GenericRepository(IContext context)
    {
       _context = context;
    }

    public Get(int id)
    {
        return _context.Set<T>().Find(id);
    }
}

Then the service class looks like this:

public EmployeeService(IGenericRespository<Employee> employee) 
{
    this.employee = employee;
}

The problem with the generic repository is that you have to create your implementations for Create, Insert, and Delete as well Fetch. This can get ugly quickly if you try to start using DbEntity and attaching entities to the context through the repository. For example if you didn't want to load the record before updating it you would have to know how to attach it and set it's state in the context. This can be tedious and troublesome because it's just not that straight forward. Once you add in some child relationships and managing child collections things really go to hell fast. If you're not needing to Mock your repositories for testing I would advise against this approach unless you find an implementation online that you understand.

With all these solutions the key is understanding how IOC works and configuring your IOC container accordingly. Depending on the container you use it can get really confusing how to register stuff, especially when generics are involved. I use Autofac whenever I can because of it's simplicity. I would never Unity except when the client insists.

The most important point when choosing your approach is to make sure it's in line with what you need. I would always say it's good to use the pattern to some degree. However if you are not writing unit tests and not needing to Mock classes for testing then you could skip the UnitOfWork and just use your IContext and skip the Repositories and just use DbSets. They accomplish the same things. As long as you are injecting things properly you get the other benefits of the pattern and the cleanliness in your design but you lose the ability to Mock the objects for testing.

Then your code would look like this which is as simple as it can get:

public EmployeeService(IContext context) 
{
     this.employees = context.Employees;
}