Model Design for MVC4 Applications using Entity Framework Code First

Question

I would like to put the following design to your consideration. This is the pattern I use to follow when I'm building a Model and Data Access Layer for an MVC Application (using Entity Framework 5). I'm looking for any advises about how to improve me design (feel free to provide a new one) because I feel that it can be done better.

I want to focus on a model to be consumed for MVC Applications using Entity Framework 5 Code First, because this way we have a more specific scenario.

First I have the interface IEntity. All entities in my model implement it and this way is ensured that all entities have a numeric key.

interface IEntity
{
    int Id { get; set; }
}

Next I define entities like this:

class Person : IEntity
{
    public virtual int Id {get;set;}
    public virtual string Name {get;set;}
    public virtual int Age {get;set;}

    public virtual int PetId {get;set;}
    public virtual Pet Pet {get;set;}
}

class Pet : IEntity
{
    public virtual int Id {get;set;}
    public virtual string Name {get;set;}

    public virtual int OwnerId {get;set}
    public virtual Person Owner {get;set}
}

This is the important part:

I use a mix between Unit of Work and Repository to build a Data Access layer that let me decouple the Business logic from my data.

The abstract definition of a Unit of Work is like this:

public interface IUnitOfWork
{
    int SaveChanges();
}

In this case I need a repository for People and other for Pets.

interface IPersonRepository
{
    // Here I declare each method that my application needs.
    // Each database dependent implementation will implement those methods.
    void Add(Person person);
    void Edit(Person person);
    void Remove(Person person);
    void RemoveById(int id);
    IQueriable<Person> GetByName(string name);
}

interface IPetRepository
{
    // Similar to IPersonRepository
}

Now in my case I build a concrete implementation of my repositories and UoW using Entity Framework.

public class EFUnitOfWork : DbContext, IUnitOfWork
{
    public DbSet<Person> People { get; set; }
    public DbSet<Pet> Pets { get; set; }
}

public class EFPersonRepository : IPersonRepository
{
    private readonly EFUnitOfWork context;

    public EFPersonRepository(EFUnitOfWork context)
    {
        this.context = context;
    }

    public Directory GetById(int id)
    {
        return context.People.Find(id);
    }

    // Other methods of the interface IPersonRepository.
}

public class EFPetRepository : IPetRepository
{        
    // Similar to EFPersonRepository.
}

I use Ninject as my Dependency Injection framework. In configuration I bind each abstract definition (interface) with the concrete implementation. I'm doing the binding using InRequestScope and this way all repositories that are instantiated on each Http Request share the same DbContext (through the same UnitOfWork). I'm concerned if InRequestScope really does what I'm expecting because sometimes I get an exception that says that the DbContext has been disposed.

public class NinjectControllerFactory : DefaultControllerFactory
{
    private readonly IKernel kernel;

    public NinjectControllerFactory()
    {
        kernel = new StandardKernel();
        AddBindings();
    }

    protected override IController GetControllerInstance(System.Web.Routing.RequestContext requestContext, System.Type controllerType)
    {
        return controllerType == null ? null : (IController)kernel.Get(controllerType);
    }

    private void AddBindings()
    {
        kernel.Bind<EFUnitOfWork>().To<EFUnitOfWork>().InRequestScope();

        kernel.Bind<IPersonRepository>().To<EFPersonRepository>().InRequestScope();
        kernel.Bind<IPetRepository>().To<EFPetRepository>().InRequestScope();
    }
}

And that's all.

Remarks

Pros:

• I can replace my concrete implementation of the data access by implementing a new set of repositories and configure the bindings.
• The business logic only see the methods exposed in repositories. This prevent using complex queries outside the data access layer.

Cons:

• The master object (a mean the context in case of EF) that has all functionality remains hidden. So we don't have any information about tracking entities, etc, in the code outside the repository. Sometimes that info is useful when debugging.
• Each query to the data must be declared in the abstract definition of the repository and then implemented on all concrete implementation. Which I think is good but someone may think that this is tedious.

Answer 1

I've used this kind of approach myself in the past and it's worked well. However lately I've been considering whether it's overkill. Considering Entity framework is already following the repository and unit of work pattern you are essentially just adding the same pattern over the same pattern and that may potentially not give you any extra benefit.

I've tended to find myself using more of a service architecture while still using dependency injection etc to manage the dependencies into my various classes. I've found it's meant less layers without losing any benefits of abstraction and TDD. It will still allow you to decouple your business logic from the data access whilst not having that extra layer of complexity.

I'm also not sure about enforcing all your entities to requiring an Id. There are plenty of situations when Id's are not required on model table definitions so to enforce such a restriction seems to me potentially limiting? Situations such as link tables, or where a table might have a composite primary key rather than just an Id field.

In saying all that, nice implementation. Seems to follow conventions of the patterns you are hoping to achieve.

As requested here's an example of abstracting away EF as much as I was able whilst not requiring a seperate repository layer and using a service type approach instead (don't sue me for spelling errors :))

NOTE: Example was using Code first methodology of Entity Framework.

public interface IDataContext
{
    IDbSet<Address> Addresses { get; set; }
    IDbSet<Contact> Contacts { get; set; }

    System.Data.Entity.Database Database { get; }
    DbEntityEntry<TEntity> Entry<TEntity>(TEntity entity) where TEntity : class;
    int SaveChanges();
}

public class MyDataContext : DbContext, IDataContext
{
    public IDbSet<Address> Addresses { get; set; }
    public IDbSet<Contact> Contacts { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        modelBuilder.Conventions.Remove<PluralizingTableNameConvention>();
        modelBuilder.Conventions.Remove<ManyToManyCascadeDeleteConvention>();
    }
}

Using a service layer is primarily where I will now store my business logic

public class BaseService
{
    protected readonly IDataContext DataContext;
    public BaseService(IDataContext dataContext) { DataContext = dataContext; }     
}

Potentially for mocking I might consider creating an interface for each service I create

public interface IAddressService
{
    IEnumerable<Address> All(int id);
    Address GetById(int id);
    IEnumerable<Address> GetAddressWithinRange(string street, int rangeInMetres);
}

public class AddressService : BaseService, IAddressService
{
    public AnalogService(IG5DataContext dataContext) : base(dataContext)
    {
    }

    public IEnumerable<Address> All()
    {
        return DataContext.Addresses.Where(p => !p.Deleted).ToList();
    }

    public Address GetById(int id)
    {
        return DataContext.AnalogInputs.Find(id);
    }

    public IEnumerable<Address> GetAddressWithinRange(string street, int rangeInMetres)
    {
        return DataContext.Addresses
            .AsQueryable()
            .Where(p => p.Street == street && p.DistanceFromCentre < rangeInMetres);
            .ToList();
    }

    // Other business methods here

    private IQueryable<Address> AsQueryable(Machinery machinery)
    {
        return DataContext.Entry(machinery).Collection(v => v.AnalogReadings).Query();
    }
}

Then using an IOC container (Ninject, AutoFac, Unity are a few I've used. Or see this blog by Scott Hanselman for a list of what's around) I would inject these into my controller. The setup of the dependency registration would be dependant on the IOC implementation.

public class AddressController
{
    private readonly IDataContext _dataContext;
    private readonly IAddressService _addressService;

    public AddressController(IDataContext dataContext, IAddressService addressService)
    {
        _dataContext = dataContext;
        _addressService = addressService;
    }

    [HttpGet]
    public ActionResult Index()
    {
        var addresses = _addressService.All();

        return View("Index", addresses);
    }

    [HttpGet]
    public ActionResult Details(int addressId)
    {
        var address = _addressService.GetById(id);

        if(address == null)
            return HttpNotFound();

        return View("Address", address);
    }

    [HttpPost]
    public ActionResult Details(Address address)
    {
        var model = _addressService.GetById(address.Id);

        if(address == null)
            return HttpNotFound();

        // I might consider using A mapping framework like AutoMapper here.  Not sure if this is correct
        // syntax but hopefully you get the point
        Mapper.Map(address, model);         
        _dataContext.SaveChanges();

        return RedirectToAction("Details");
    }
}