Brent
Brent

Reputation: 4876

MVC repository architecture and accessing different tables

Thank you for helping me understand some of this stuff:

Say I have 2 controllers in an MVC application - 1 controls viewModels related to salespeople 1 controls viewModels related to sales

Each have a their own repository, which access data using Entity framework (code first)

both repositories are set up to handle dependency injection, but also have 0 argument constructors with defaults to use the appropriate EF dataAccess.

the salespeople controller uses a _salesPeopleRepository.getAllSalesPeople() function which returns a List of salespeople to populate an index view.

the sales controller needs to access the same list to populate a dropdownlist.

there are several ways to get the information to the sales controller, and I was wondering which options would be considered best practice:

a) In the controller

db = new DataContext();
_saleRepos = new SalesRepository(db);
_salesPeople = new SalesPeopleRepository(db);
 .....
modelA.SalePeopleSelectList = SelectList(_salesPeople.getAllSalesPeople(),"id","name")

b) in the SalesRepository - either using EF itself:

public IEnumerable<salesPerson> getAllSalesPeople()
{ 
    return _db.SalesPeople.ToList();
}

c) or instantiating and injecting the same data access object before calling the function

public IEnumerable<salesPerson> getAllSalesPeople()
{ 
    return (new SalesPersonRepository(_db)).getAllSalesPeople();
}

Edit

If the answer is a), how should custom buisiness logic be called from 1 repository - say for instance sales has a storeId and the repository checks that the storeId entered for the sale matches the storeId for the salesPerson. Should the salesPerson object used for buisness logic purposes (within the salesRepository) be accessed via the salesPerson repository, or directly from the dataContext object?

Thank you for your thoughts and experience

Upvotes: 3

Views: 752

Answers (3)

Dennis Traub
Dennis Traub

Reputation: 51634

Best Practice always depends on the context, but a combination of the Repository and Unit Of Work patterns are regularly used on top of EF.

Usage:

using (var uow as new DataContext()) {
    var salesPeople = new SalesPeopleRepository(uow);

    // ...

    uow.Commit(); // If changes must be committed back to the database
}

Implementation:

public interface IUnitOfWork {
    public void Commit();
}

public class DataContext : IUnitOfWork {
    public void Commit() {
        this.SaveChanges();
    }
}

public class SalesPeopleRepository {

    private DataContext _db

    public SalesPeopleRepository(IUnitOfWork uow) {
        _db = uow as DataContext;
    }

    public IEnumerable<SalesPerson> GetAllSalesPeople() { 
        return _db.SalesPeople.ToList();
    }
}

Upvotes: 2

cuongle
cuongle

Reputation: 75306

First, the C# naming convention should be followed: getAllSalesPeople() should be GetAllSalesPeople. Second, IoC Container and dependency injection would be the best practice in this case.

The item a should be avoied because DataContext and Repositories are created directly in controller, it is violating dependency injection and make your code tight coupling with Repositories and DataContext and no way to mock for unit testing. Instead, repositories should be injected into controllers and DataContext should be injected into Repositories.

public Repository(DataContext dataContext)
{
    _dataContext = dataContext;
}

public SalesController(ISalesRepository salesRepository, 
                          ISalesPeopleRepository salesPeopleRepository) 
{ 
   _salesRepository = salesRepository; 
   _salesPeopleRepository = salesPeopleRepository; 
} 

The lifetime management for DataContext should be kept in per request in IoC Container instead of creating directly in controller, most of IoC Container supports this. Don't know which IoC container you use, but my faves is: Autofac and Windsor.

For the item c, you are making the business logic leak to Repository layer, instead, business logic should be in controller or in separate layer. Repository just deal with CRUD operation with your database.

Upvotes: 1

Garrett Vlieger
Garrett Vlieger

Reputation: 9494

It doesn't make sense to have your SalesRepository retrieving data from the SalesPerson table. That data access logic ought to be centralized in the SalesPeopleRepository. Duplicating the method across repositories will just muddy the water, in my opinion.

So why not have both the SalesRepository and SalesPeopleRepository used in your Sales Controller? I would just instantiate an instance of the SalesPeopleRepository and use the method already defined in there.

Also, if your Controllers are using dependency injection, you could just have the repositories passed in to the constructor:

public SalesController (ISalesRepository salesRepository, ISalesPeopleRepository salesPeopleRepository)
{
   this._salesRepository = salesRepository;
   this._salesPeopleRepository = salesPeopleRepository;
}

Upvotes: 5

Related Questions