Why it's not a good idea to pass entities as Models in MVC?
We're developing a pretty large application with MVC 2 RC2 and we've received some feedback on the way we're using the Entity Framework's Lazy Loading.
We're just getting the entities in the controller and sending them as models to the Views, that is causing that the view code asks the Database for the navigation properties we are using in it. We have read about this and it appears is not a good design, but we were wondering why?
Can you help us understand this design problem?
The main issue here is coupling. The idea behind a model, which is the "M" in "MVC", is that it has no external dependencies. It is the "core" of your application. The dependency tree of a well-designed app architecture should look something like this:
+---------------------------------------> Views | | | | | v Controllers ----+-> Model Transformer -----> View Model | \ | | \ | v \ v Data Access <---- Persistence --------> Domain Model | / | / v / Mapper ------+
Now I realize it's not exactly convincing to just say "here's an architecture, this is what you should use", so let me explain what's happening here:
- Controller receives a request.
- Controller calls out to some kind of persistence layer (i.e. repository).
- Persistence layer retrieves data, then uses a mapper to map to a domain model.
- Controller uses a transformer to change the domain model into a view model.
- Controller selects the necessary View and applies the View Model to it.
So, why is this good?
The domain model has no dependencies. This is a very good thing, it means that it's easy to perform validation, write tests, etc. It means that you can change anything else anywhere in your architecture and it will never break the model. It means that you can reuse the model across projects.
The persistence layer returns instances of the domain model. The means that it can be modeled as a totally abstract, platform-agnostic interface. A component that needs to use the persistence layer (such as the controller) does not take on any additional dependencies. This is ideal for Dependency Injection of the persistence layer and, again, testability. The combination of persistence, data access, and mapper can live in its own assembly. In larger projects you might even be able to further decouple the mapper and have it operate on a generic record set.
The Controller only has two downstream dependencies - the domain model and the persistence layer. The model should rarely change, as that is your business model, and since the persistence layer is abstract, the controller should almost never need to be changed (except to add new actions).
The Views depend on a separate UI model. This insulates them from changes in the domain model. It means that if your business logic changes, you do not need to change every single view in your project. It allows the views to be "dumb", as views should be - they are not much more than placeholders for view data. It also means that it should be simple to recreate the view using a different type of UI, i.e. a smart client app, or switch to a different view engine (Spark, NHaml, etc.)
Now, when using O/R Mappers such as Linq to SQL or Entity Framework, it is very tempting to treat the classes they generate as your domain model. It certainly looks like a domain model, but it is not. Why?
The entity classes are tied to your relational model, which over time can and will diverge significantly from your domain model;
The entity classes are dumb. It is difficult to support any complex validation scenarios or integrate any business rules. This is referred to as an anemic domain model.
The entity classes have hidden dependencies. Although they may appear to be ordinary POCOs, they may in fact have hidden references to the database (i.e. lazy loading of associations). This can end up causing database-related issues to bubble up to the view logic, where you are least able to properly analyze what's going on and debug.
But most importantly of all, the "domain model" is no longer independent. It cannot live outside whatever assembly has the data access logic. Well, it sort of can, there are ways to go about this if you really work at it, but it's not the way most people do it, and even if you pull this off, you'll find that the actual design of the domain model is constrained to your relational model and specifically to how EF behaves. The bottom line is that if you decide to change your persistence model, you will break the domain model, and your domain model is the basis for just about everything else in your app.
Entity Framework classes are not a domain model. They are part of a data-relational model and happen to have the same or similar names that you might give to classes in a domain model. But they are worlds apart in terms of dependency management. Using classes generated from an ORM tool as your domain model can only result in an extremely brittle architecture/design; every change you make to almost any part of the application will have a slew of predictable and unpredictable cascade effects.
There are a lot of people who seem to think that you don't need a cohesive, independent domain model. Usually the excuse is that (a) it's a small project, and/or (b) their domain model doesn't really have any behaviour. But small projects become large, and business rules become (far) more complicated, and an anemic or nonexistent domain model isn't something you can simply refactor away.
That is in fact the most insidious trait of the entities-as-model design; it seems to work fine, for a while. You won't find out how much of a mistake this is until a year or two down the road when you're drowning in defect reports and change requests while desperately trying to put together a real domain model piecemeal.
One potential issue with this design is that the view might iterate through the model objects (that are lazy loaded) more than once and cause an unnecessary overhead. For instance, if a Web page displays the same data in two different forms in a couple of different locations in the page, it'll loop through the query twice and causes two round-trips to the database (Look at the tags under the question and in the sidebar on this page and assume they came from a single query). The view could deal with this problem by caching the results once and loop twice on the cached data, but this is not what a view should deal with. It should present the data given to it without worrying about these stuff.
The problem is that your UI is tied more directly to your entity than necessary.
If your entity is encapsulated by a ViewModel, then your UI can not only contain the entity (the data it wishes to eventually save), but it can also add more fields and more data that can be used by the controller to make decisions, and be used by the View to control the display. In order to pass the same data around outside of a ViewModel would require that you use action method parameters and ViewData constructs, which does not scale, especially for complex ViewModels.
The view is stupid and ignorant. It should not and doesn't want to know anything. Its very shallow and focusses only on display. This is a good thing, as the view does what it does best.
By doing it your way, you leak, what should be data concerns, to the view, and furthermore you limit you view only to recieve data from your entity as the strongly typed model.
Furthermore you let you dependency on EF go all the way to the view and penetrates you app, where you should try to be as loosely coupled to that dependency as you can.