Reputation: 4876
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
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
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
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