8

I think I've hit that "paralysis by analysis" state. I have an MVC app, using EF as an ORM. So I'm trying to decide on the best data access pattern, and so far I'm thinking putting all data access logic into controllers is the way to go.. but it kinda doesn't sound right. Another option is creating an external repository, handling data interactions. Here's my pros/cons:

If embedding data access to controllers, I will end up with code like this:

using (DbContext db = new DbContext())
{
    User user = db.Users.Where(x=>x.Name == "Bob").Single();
    user.Address.Street = "some st";
    db.SaveChanges();
}

So with this, I get full benefits of lazy loading, I close connection right after I'm done, I'm flexible on where clause - all the niceties. The con - I'm mixing a bunch of stuff in a single method - data checking, data access, UI interactions.

With Repository, I'm externalizing data access, and in theory can just replace repos if I decide to use ado.net or go with different database. But, I don't see a good clean way to realize lazy loading, and how to control DbContext/connection life time. Say, I have IRepository interface with CRUD methods, how would I load a List of addresses that belong to a given user ? Making methods like GetAddressListByUserId looks ugly, wrong, and will make me to create a bunch of methods that are just as ugly, and make little sense when using ORM.

I'm sure this problem been solved like million times, and hope there's a solution somewhere..

And one more question on repository pattern - how do you deal with objects that are properties ? E.g. User has a list of addresses, how would you retrieve that list ? Create a repository for the address ? With ORM the address object doesn't have to have a reference back to user, nor Id field, with repo - it will have to have all that. More code, more exposed properties..

Improve this question
10
  • Check the answer on this question : stackoverflow.com/questions/458146/… Commented Mar 17, 2011 at 13:26
  • See this question..stackoverflow.com/questions/3720013/… Commented Mar 17, 2011 at 13:27
  • Search for the IRepository pattern here on SO...related to EF.. you'll find very similar implementations Commented Mar 17, 2011 at 13:28
  • +1, because right now I'm using a generic repository pattern + these GetAddressListByUserId ugly methods + some others generic methods. Commented Mar 17, 2011 at 13:29
  • 1
    I think there's a strong headwind developing for performing data access directly in your controllers (and other top-level callers). For example, see this series of articles on The Onion Architecture, the classic (if controversial) "Repository is the New Singleton", and The evils of the repository abstraction layer. Commented Mar 17, 2011 at 14:17

3 Answers 3

Reset to default
7

The approach you choose depends a lot on the type of project you are going to be working with. For small projects where a Rapid Application Development (RAD) approach is required, it might almost be OK to use your EF model directly in the MVC project and have data access in the controllers, but the more the project grows, the more messy it will become and you will start running into more and more problems. In case you want good design and maintainability, there are several different approaches, but in general you can stick to the following:

Keep your controllers and Views clean. Controllers should only control the application flow and not contain data access or even business logic. Views should only be used for presentation - give it a ViewModel and it will present it as Html (no business logic or calculations). A ViewModel per view is a pretty clean way of doing it.

A typical controller action would look like:

public ActionResult UpdateCompany(CompanyViewModel model)
{
    if (ModelState.IsValid)
    {
        Company company = SomeCompanyViewModelHelper.
                          MapCompanyViewModelToDomainObject(model);
        companyService.UpdateCompany(company);
        return RedirectToRoute(/* Wherever you go after company is updated */);
    }
    // Return the same view with highlighted errors
    return View(model);
}

Due to the aforementioned reasons, it is good to abstract your data access (testability, ease of switching the data provider or ORM or whatever, etc.). The Repository pattern is a good choice, but here you also get a few implementation options. There's always been a lot of discussion about generic/non-generic repositories, whether or not one should return IQueryables, etc. But eventually it's for you to choose.

Btw, why do you want lazy loading? As a rule, you know exactly what data you require for a specific view, so why would you choose to fetch it in a deferred way, thus making extra database calls, instead of eager loading everything you need in one call? Personally, I think it's okay to have multiple Get methods for fetching objects with or without children. E.g. 

public class CompanyRepository
{
    Get(int Id);
    Get(string name);
    GetWithEmployees(int id);
    ...
}

It might seem a bit overkill and you may choose a different approach, but as long as you have a pattern you follow, maintaining the code is much easier.

Improve this answer
Sign up to request clarification or add additional context in comments.

21 Comments

"Ease of switching ORMS" is a terrible reason to advocate for the Repository pattern. 1. It doesn't happen often. 2. There are enough differences between them that you are probably looking at a large refactor anyway.
@Yakimych - Why not? Abstractions are for when implementations may change or you want to hide implementation details. Chances are a system written to interact with a relational db will not ever change to not using a relational db. One layer isn't a good reason at all either. If all my db access is done via controllers thats still one layer to change.- Also wanted to add that your "repository" isn't a repository pattern at all if you have GetWithEmployees(). All you are doing is writing a DAL with a fancier name. ;)
For starters, I didn't say "The Repository pattern increases maintainability". I said you would probably not want data access in your controllers if you want long-term maintainability for large projects (see one of previous comments). In this case the repository pattern is just referred to as one of the ways to break out the data access code. In any case, maintainability increases because your data access code, business logic and presentation logic are not blended into one piece of code. We usually try to make it clean and have Web, Services, Repositories layers. (And no 1500 line repos ;))
Furthermore, having data access in controllers rather severely limits your data access code reusal. Not the rarest of cases is when you have a Web app and then want to create a windows app which essentially does the same. In this case you'll be copypasting most of your data access code from your webapp instead of reusing existing methods from the services or repo assembly.
@Yakimych That is one of the reasons I want to externalize db calls - there's a fair chance we'll have to provide a web service that does same thing. We could still use MVC for that, I guess, returning JSON, but not sure if that's the best way to do it.
|
5

Personally I do it this way:

I have an abstract Domain layer, which has methods not just CRUD, but specialized methods, for example UsersManager.Authenticate(), etc. It inside uses data access logic, or data-access layer abstraction (depending on the level of abstraction I need to have).

It is always better to have an abstract dependency at least. Here are some pros of it:

  • you can replace one implementation with another at a later time.
  • you can unit test your controller when needed.

As of controller itself, let it have 2 constructors: one with an abstract domain access class (e.g. facade of domain), and another (empty) constructor which chooses the default implementation. This way your controller lives well during web application run-time (calling empty constructor) and during the unit-testing (with mock domain layer injected).

Also, to be able to easily switch to another domain at a later time, be sure to inject the domain creator, instead of domain itself. This way, localizing the domain layer construction to the domain creator, you can switch to another implementation at any time, by just reconstructing the domain creator (by creator I mean some kind of factory).

I hope this helps.

Addition:

  • I would not recommend having CRUD methods in domain layer, because this will become a nightmare whenever you rich the unit-testing phase, or even more, when you need to change the implementation to the new one at a later time.
Improve this answer

6 Comments

+1 - see also data access object. It uses a repository to perform the queries you need.
So you have an abstract repository for each class that's data driven ?
What do you mean by class that's data driven? - controller?
He probably meant domain classes.
Thanks Yakimych! Actually, I do an abstract repository for each domain data class - yes. But it would be equally effective to have a direct data access code inside domain data classes for the simplicity. All depends on the project size and extensibility requirements. The main thing is - have everything injected into controller - and this will make your life better... well, at least repository :-)
|
0

It really comes down to where you want your code. If you need to have data access for an object you can put it behind an IRepository object or in the controller doesn't matter: you will still wind up with either a series of GetByXXX calls or the equivilent code. Either way you can lazy load and control the lifetime of the connection. So now you need to ask yourself: where do I want my code to live?

Personally, I would argue to get it out of the controller. By that I mean moving it to another layer. Probably using an IRespository type of pattern where you have a series of GetByXXX calls. Sure they are ugly. Wrong? I would argue otherwise. At least they are all contained within the same logical layer together rather than being scattered throughout the controllers where they are mixed in with validation code, etc.

Improve this answer

3 Comments

-1 'for the 'or in the controller doesn't matter.' It does matter. A class should have only one reason to change. If the controller is doing two things, then it has two reasons to change, and that's bad design.
Well said George. Ken, see also en.wikipedia.org/wiki/Single_responsibility_principle
@George Stocker - Controllers by their very nature violate SRP. Big repositories/services violate SRP as well. You can change a repository/service layer because GetCustomer needs additional fields because and then GetCustomerBalanceByDateRange needs to change as well. Repos/Services become monstrous towers of SRP violations when each class dependent on it uses only a fraction of its functionality.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.