views:

283

answers:

4

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?

Thanks!

A: 

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.

Mehrdad Afshari
Could this design be justified if the view shows a lot of data that if eager loaded would cause EF (or any ORM) to generate a HORRIBLE and slow SQL? This has been the case with many of our views...
sabanito
@sabanito: In real world, every design can be justified when you add cost, time, and resources factors to it! There may be better ways to accomplish that (creating more specific ViewModels for your views) but that may add too much code and may be infeasible in your circumstances. All you can say is that given infinite resources, it's probably not the best way to accomplish things.
Mehrdad Afshari
@sabanito: If EF generates HORRIBLE and slow SQL, it is only your fault. Don't try to load everything in one query using `Include`, but divide it into more than one, so 1 to n relations are not loaded in one sql statement. Eager loading doesn't make it any slower, specially when you need all data and still load everything with lazy loading.
LukLed
+1  A: 

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.

womp
We've used some ViewModels but only in the views that combine data from different entities. What do you mean with ViewData constructs?
sabanito
I just mean passing complex objects in ViewData, which are referenced by magic strings... usually controlled by some kind of home-grown framework to keep track of them all.
womp
+1  A: 

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.

Luhmann
So you mean that every view has to receive a ViewModel class in it's model property? Even if it's pretty simple?
sabanito
I definitely prefer that. It consistent and concerns are nicely seperated.
Luhmann
+10  A: 

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:

  1. Controller receives a request.
  2. Controller calls out to some kind of persistence layer (i.e. repository).
  3. Persistence layer retrieves data, then uses a mapper to map to a domain model.
  4. Controller uses a transformer to change the domain model into a view model.
  5. 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.

Aaronaught
It's not correct that EF ties you to a relational model, the generated classes tie you to the conceptual model, and on v3.5 also to their API, but v4.0 supports POCO. This model can be used on the application, there's no need for an additional one, that's the purpose of EF.
Max Toro
@Max Toro: Just because it's a POCO does not make it a domain model. You still have to design it around the relational model in order to make it fit. Show me an example of an EF mapping where the mapped model is significantly different from the relational.
Aaronaught
@Aaronaught I don't have much experience with EF, but it's supposed to support a lot of mapping scenarios, like inheritance, complex properties (eg. 2 columns map to 1 property). The most significant to me is removing the foreign keys from your classes. There are a lot of changes, that's why it has it's own query language, ESQL.
Max Toro
@Max Toro: Even if you *could* perform a perfect mapping to a real domain model using EF (and EF is nowhere that point yet, even NHibernate isn't perfect), that does not eliminate the hidden dependencies, which are a serious problem. Lazy loading can introduce bugs; eager loading can introduce major performance issues. There needs to be a domain-specific mapping layer that decides what to do based on the context in which it is being used.
Aaronaught
@Aaronaught You mean a loading strategy based on the controller's needs? I agree that's much better than lazy loading on the view. This can be enforced by disabling lazy loading.
Max Toro
@Max Toro: Simply disabling lazy loading across the board would kill performance. The most efficient queries for large data sets usually involve returning multiple result sets and actually using the FK IDs (what EF tries to hide from you) to put them together in the mapping layer. This isn't the kind of logic that should be in a controller; it should be in an intelligent query-mapping layer (i.e. repository). This is just one of many areas where heavy reliance on an ORM starts to break down; I've personally experienced dozens, maybe hundreds.
Aaronaught
@Aaronaught Why disabling lazy loading kills performance? I'm not saying eager load everything, just what you need. Anyway, the topic is not about performance, but coupling. I agree a service layer is needed, which in my view can return the same model generated by EF.
Max Toro
No, if the model was *generated* by EF then it suffers from **all** of the problems I originally described. If you have a complex mapping to POCO classes then you *might* be able to mitigate *some* (but not all) of those problems.
Aaronaught
@Aaronaught Sorry I meant POCO model. I understand the coupling issue, but not the loading issue, what's wrong with eager loading?
Max Toro
@Max Toro: Targeted eager loading, where you specify *exactly* which associations are to be eagerly fetched for a *specific* query, are fine. Eager-loading all entities in the graph would be a disaster; you'd end up `JOIN`-ing just about every table in the database.
Aaronaught
@Aaronaught Of course, eager loading based on a strategy. So the issues are mapping, coupling and loading. I think EF (at least v4) offers a decent solution for all three.
Max Toro
@Max Toro: EF 4 does not offer anywhere near a complete solution. It has most of the hooks you need to *create* a complete solution, but not without another layer sitting on top of it that decides how to construct the right domain model for a specific scenario. The alternative is for this logic to be in your UI/controller, and you definitely do *not* want it there.
Aaronaught
@Aaronaught Wow, thanks for the incredibly detailed answer. I think I've got the point of the coupling, but isn't it a lot of work creating classes that just mimic your UI needs? Do you use something like Automapper or something that "converts" your Poco/Entity classes to UI ones? I definitely agree and would pay the time price with complex views, but there are times when your view shows exactly what your POCO/entity has in its properties (catalog entries)
sabanito
@sabanito: It may seem like a lot of work, but when you consider the typical breakdown of development work (more than 80% of which is design, refactoring, and debugging), it's really not, especially since EF can generate the entity classes for you - you only need to "write" the domain and view models, which you'd eventually need to write anyway. AutoMapper is a great tool to employ when some part of your domain model happens to be identical to the relational model; it gets you up and running quick, but you still have the ability to change the mapping later.
Aaronaught
@Aaronaught Of course, a service layer is needed, controllers should not call the ObjectContext directly. My point is EF has everything you need so you can have a single model (domain) and not 2 models (relational and domain). Else, there's no point of using a product as advanced/complex as EF.
Max Toro
@Max Toro: For the most part I agree, but now you have one of two scenarios. Either (a) your service layer and EF design both map to POCO model classes in a separate assembly, in which case you basically just *built your own domain model*, or (b) your service layer returns entity classes in the same assembly as the `ObjectContext`, which means any consumer of the service layer takes on the additional dependency of the EF assembly, which is one of the main issues I pointed out above.
Aaronaught
Too bad I can only upvote this answer once! It would be nice to see a complete example of how you would design/implement such a system (especially the EF/DomainModel parts and the mapping inbetween).
M4N
I wish I could upvote this answer at least two or three more times.
Brant Bobby