7

I've encounter yet again this situation where programmers are using autogenerated models (from LinqToSql, EF, nHibernate, whatever) as view models.

Usually the architecture is very simple :

  1. Some kind of DAL containing only EF or LinqToSql
  2. Some modules for very specific stuff
  3. MVC project

It may or may not contain a repository. Either way, DAL models are used as View Models. The number of properties used from said model vary from 10% to 90% per view, although usually less than 75% properties are used.

Recently I've encountered a new mutation of said pattern, where when DTO model lacks a required additional field, a programmer creates a view model inheriting EF model and pass it to the view.

Are there tutorials pushing for this approach? Or some books or 'trainers' pushing it? Is there any sensible reason beside "I'm lazy"? Am I under totally wrong impression and this is correct and accepted pattern?

  • I think this is the default implementation of MVC, unless I misunderstand you. Are you concerned about the lack of a distinct "ViewModel" layer between models and view? – JacquesB May 4 '17 at 14:20
  • 1
    What you call lazy I call KISS (keep it simple) and DRY (don't repeat yourself). You should only introduce an additional layer like a VM layer if you actually need it. As for db queries pulling unnecessary data - this has nothing to do with the VM layer. Of course when you need a VM layer, use it. – JacquesB May 4 '17 at 14:50
  • 1
    @JacquesB That's all well and good, but the reality is that your coupling your data model with your UI. For small, trivial apps you can get away with that, but even moderately complex applications this leads to a maintainability nightmare which is much harder to undo than it is to do it properly separated to begin with. – Andy May 5 '17 at 0:40
  • 1
    @JacquesB And my point is that I've yet to build an application that has ever been as simple as you suggest, and I've been doing this for over 20 years now. Even if it starts simple, apps quickly gain complexity, and its far easier to plan for complexity you are certain is coming than to break things up later. – Andy May 5 '17 at 14:49
  • 1
    @JacquesB And no, the UI should NOT mimic the data layer. The UI should mimic the use cases and the behavior required in the use case. Any data the business model has is only because its required to fulfil the behavior. Some use cases may not need all fields of a certain record in the DB, and in some use cases the same record may be considered valid in one and invalid in another. I've never been asked to build an application where there was no need for a VM layer (and really a separate Business layer). And the ones I've inherited which lacked them were a nightmare to maintain. – Andy May 5 '17 at 14:57
6

From a security standpoint you should use a 1 to 1 view to view model and only pass the properties that the view needs. If you use the full ORM class then a malicious user could manually post extra items and modify data that you did not intend to modify (baring any extra validation server side that would not be needed if the property was not available in the first place). By only passing the data required for the view you eliminate this possible attack vector by not exposing the extra parameters.

Along with the security aspect having view models separate from the ORM classes presents a clear interface between things. In Razor (.net) if you were to change an ORM class in a way that created an error it would only show up at run time where as if there was a mapping that occurred between the ORM and View Model (manual or automatic) it would have a better chance to present as a compile error.

Yes there are some times when this will create duplicate classes but it is worth the duplication because the classes could change independently for different reasons.

  • The security aspect is what really hit home for me. Anyone working in government, healthcare or finance should read this answer carefully. – Greg Burghardt May 5 '17 at 3:03
  • I don't get this at all - the purpose of the view and controller is to expose the data and operations which are allowed in the UI. – JacquesB May 6 '17 at 10:04
  • @JacquesB The View only controls what a user can do via a browser; so consider the case where an HTTP POST request is submitted to the web service using some other tool, such as Postman. If the ASP.NET controller accepted a complete EF DTO model in the body of an [HttpPost] action, then to ensure security, you would also need extra code in the action to decide which parts of the HTTP POST body to ignore, and also to make sure that this is kept up-to-date if the EF model has any new properties added in future too. – Ben Cottrell May 6 '17 at 11:04
  • @BenCottrell: If you expose a web service which allows anybody to update arbitrary data in the backend, then you you have serious problems - but this have nothing to do with MVC, which is a pattern for structuring user interfaces. – JacquesB May 6 '17 at 11:46
  • @JacquesB Indeed, this has nothing to do with the MVC pattern, but the ASP.NET MVC framework where Controllers are also responsible for exposing POST/PUT actions. Unfortunately the tutorials on the ASP.NET website do nothing to discourage beginners from exposing Entity Framework models in POST actions. – Ben Cottrell May 6 '17 at 18:32
8

There are no clear standards on how to structure the "M" layer of an MVC application. That being said, I have consistently come across problems with using business classes (or domain models, or ORM models) in the view layer of an application.

  • Display logic sneaks into business classes
  • Display logic gets peppered all over view templates
  • No clear indication to the programmer of what is absolutely necessary to generate a view
  • Presentation specific validations end up polluting classes that are not meant to be about the presentation
  • Business classes get modified to allow invalid data according to business rules, because the class is used to render a form (and therefore must allow invalid data to be entered)

    This is the big reason to separate view models from business classes!

For this reason I almost always create view models for my views -- about 99% of the time. There isn't much extra effort to create a view model, and it gives you a place to put presentation specific logic. Having that separation in place from day 1 prevents an imminent refactoring later when the needs of the user interface expand.

This refactoring happens. Every. Single. Time.

Even if your business classes and view models have exactly the same properties, it's OK to copy data. It's not OK to copy logic. And it's not OK to mix presentation logic with business logic (or data storage logic).

Separating view models and business classes boils down to Separation of Concerns and is just plain good object oriented design.

In frameworks like ASP.NET MVC using Razor templates, you don't get compiler errors in your views when you rename a property or class name, which complicates refactoring. If you have a view model, your business classes can be refactored as you see fit, and you'll get compiler errors in your view models for incompatible changes.

If you used your business classes directly in your views, then you get a run time failure, which is harder to debug and takes longer to unravel.


Concerning YAGNI (You Aren't Gonna Need It)

The YAGNI principle tells us not to build things that are unnecessary. The problem is, user interfaces and business rules undergo a lot of change over the life of an application. Separating the two into their own classes is not violating YAGNI. Remember to not just think about "Today". Things are going to change, and separating view models from business classes gives you a buffer zone between these two kinds of changes.

  • I don't see why YAGNI shouldn't apply here? If you have a very simple app which just displays records from a database, then you don't need a separate view-model layer, it is fine to use the entities as the model in MVC. If it turns out later you get more complex requirements, then you introduce the view-model layer when you need it. – JacquesB May 4 '17 at 15:18
  • +1, but you so totally get compile errors in razor views if you're model binding correctly. – RubberDuck May 4 '17 at 21:26
  • @JacquesB If you know its not going to STAY a simple app, adding the VM layer up front is the smarter choice. – Andy May 5 '17 at 0:42
  • 4
    @Andy: I had a boss tell me once, "Nothing is ever easy." And boy was he right. I have yet to find a truly simple app. There is always a hidden complexity. Too many times I've seen developers make the mistake of thinking a web app with a handful of pages is "simple." Nothing is ever easy. – Greg Burghardt May 5 '17 at 3:09
2

Being lazy can be a perfectly valid point because:

  • Producing more code makes more code to maintain, which is quite sad if it is totally unnecessary (YAGNI: You Aren't Gonna Need It)
  • If your ORM uses caching, it will work better with full object than query selecting specific columns in tables, those won't be cached so you will very likely end up querying your base more often.
  • Inheriting the DTO for the view avoids copy / pasting

Of course that is not free it costs some tighter coupling and some drawback like you will query two objects of different type (without composition/aggregation link between them) separately where you could have a join if your view's object is separated from the DTO's one.

  • Maybe I'm too used to working on stuff requiring horizontal scaling, cause I've never even consider caching on ORM layer. – Michał Brix May 4 '17 at 14:38
  • 1
    @MichałBrix I'am on the Java side, Hibernate for instance have 2 cached layers : one which is local to the current transaction, if you load twice the same object, you will get the same reference because it will be cached. This layers can't be disabled (and don't need to aniway), a second disabled by default, which enable cache accross separate transaction, if you do that, you should implements OptimisticLocking and be able to handle it gracefully. – Walfrat May 4 '17 at 14:43
0

The purpose of MVC is to separate domain logic from UI specific logic.

The standard version of MVC has three components. The View and Controller components defines the user interface (presentation and operations respectively) while the Model is some kind of domain model which is independent of any specific UI.

The MVC pattern does not say anything about what the model should be, except it shouldn't contain UI-specific logic. So using database entity objects as model is totally fine according to this.

Now to the core of your question:

Recently I've encountered a new mutation of said pattern, where when DTO model lacks a required additional field, a programmer creates a view model inheriting EF model and pass it to the view.

Well, this depends on what the additional field is! If it is some kind of domain logic (say a calculated field) which is independent of UI concerns, then it is fine. If the field is something UI specific, then it is bad, because anything UI specific should belong in the view or controller layer, not in the model layer, and by subclassing a model class, this field becomes tightly coupled to the model.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.