Always use ViewModel pattern in MVC? [closed]
Asked Answered
C

5

7

In MVC in the controller you should get the Model from DB and convert it to a ViewModel before sending it to the View. Typically using something like Automapper.

My question is, if you need to show all the properties of the Model in the view as they are, is creating a ViewModel worth it?

If the Model and ViewModel need to be the same, creating a ViewModel creates some security or benefits in the app or we are only adding unnecessary complexity?

Cacique answered 18/5, 2014 at 14:55 Comment(2)
if you expect the website to be updated over time, you should definitely use the viewmodelPremature
This question belongs on Programmers.SELymphatic
S
8

The point of using a view model is generally because your view requires more/less information than what your domain model provides.

Other benefits include decoupling your view from your domain which can cause fragility if your domain evolves.

The crux of it is, they aren't a necessity; the purpose of a view model is to provide the view with only the information it needs to render itself, if you feel a view model in your app would be redundant then don't use it. However, I would at least consider using an interface to avoid coupling.

Shied answered 18/5, 2014 at 15:13 Comment(0)
W
6

Besides what is already mentioned (separation of a concern,decoupling,etc), a separate ViewModel stops abstraction leaking that may come along with the DB Model. This is true especially if you are using EF with navigation properties enabled.

Let's say you have cars and wheels. You are showing the cars in the view.

Case 1 (No Separate ViewModel For cars): In the razor view, it is very easy to have something like below:

  public class CarModelFromDB
  {
      public string CarName{get;set;}
      //More properties
      public IEnumerable<Wheel> Wheel{get;set;}
  }  

  @model IEnumerable<CarModelFromDB>

  @foreach(var car in Model)
  {
      //some View Code
      @foreach(var wheel in car.Wheels.Where(x=>x.IsRound=true))
      {
         <span>wheel.Color</span> 
         // some View Code
      }
  }

Now, your logic for getting wheels with the cars has leaked into the view and also has enabled select N+1 situation. I don't think there is any easy ways of testing it either.

Case 2 (With ViewModel For cars): In this scenario, you can restrict the view by sending only things that it needs. It may look like below:

  public class CarViewModel
  {
      public string CarName{get;set;}
      //More properties
      public IEnumerable<string> WheelColors{get;set;}
  }  

  @model IEnumerable<CarViewModel>
  @foreach(var car in Model)
  {
      //some View Code
      @foreach(var wheelColor in WheelColor)
      {
         <span>wheelColor</span> 
         // some View Code
      }
  }

Now, your view code is very restricted in terms of what it can do, and it won't send any rogue queries to the database. Your controller is truly in control of what view is getting. You can push the wheel logic in there or ideally in some service method that gets called from the action method. Also, you can do proper testing on the action method and feel confident in your system. I hope this helps.

Update

Case 3 (dynamic ViewModel): If you are comfortable with dynamic types, you can avoid all the casting and mapping. As long as your view gets the properties it needs, it will be happy. It does not matter where those come from. So the code will be:

  public class CarViewModel
  {
      public string CarName{get;set;}
      //More properties
      public IEnumerable<string> WheelColors{get;set;}
  }  

  // pass the List<CarViewModel> to the view

  @model dynamic
  @foreach(var car in Model)
  {
      //some View Code
      @foreach(var wheelColor in WheelColor)
      {
         <span>wheelColor</span> 
         // some View Code
      }
  }

The potential downside/extra work is to make sure you have tests for the existence of those properties on the model.

Again like it is mentioned earlier, this should not be considered as the one size fit all type of solution. These are some options and used only if they make sense.

Whitener answered 18/5, 2014 at 15:59 Comment(0)
B
4

While this question will most likely be closed as primarily opinion based, it's indeed a good question. A ViewModel is almost always worth the effort in an app of marginal complexity.

Most apps will benefit from extending all Viewmodels from a BaseViewModel.cs, so that things like the CurrentTab (highlight current nav bar page) or PageTitle can be shown in the _Layout.cshtml. In that case, the viewmodel is always necessary as a derivative of BaseViewModel.

Bergmann answered 18/5, 2014 at 15:12 Comment(8)
I highly disagree about a base view model. Such things tend to become dumping grounds for every setting you might possibly want to reuse somewhere else, and before you know it it's got a ton of junk that's not necessary. If you need common behavior like that, I tend to encapsulate that in it's own class and either contain it in your model (if necessary) or make it altogether unnecessary by extracting that code into an action of its own. Inheritance is about something being an "is-a" relationship, not just a convenient way to share data.Kohn
@Erik The main purpose of a BaseViewModel is to do away with the need for ViewBag and ViewData which are terrible design choices in a strong-typed language. And common core data that every class of a certain Type needs, such as the title of a page or what the current tab is, is exactly the purpose of implementing a Base Class.Bergmann
And I disagree, and have already given my reasons. My solutions do not use ViewBag or ViewData, and do the exact same thing in a more maintainable manner without abusing inheritance as a way to share data. Sometimes base classes are necessary, but all too often people are lazy and it's the first thing they jump to, rather than using better decoupled solutions. My disagreement is the claim that "most apps will benefit" from something which is clearly a poor design choice.Kohn
@Erik If you decide that every ViewModel, no matter what it's showing, will need to be told it's current tab, that is an "is-a" relationship. Inheriting is the correct choice for having a parent class of properties that every derived object needs in order to be a complete ViewModel. That is clearly the proper design choice, and I'd encourage you to give an objective re-reading of what makes something a Parent Class versus a Utility Class.Bergmann
I disagree. Which tab i'm on has nothing to do with the view model. That is, the functionality of page. Your navigation system should be rendered as a child action, with its own model. You're mixing responsibilities and violating the Single Responsibility Principle. Base classes should only be used as a last resort, and there are many other options to do this that do not require polluting your models with base classes.Kohn
The _Layout.cshtml needs to access that value in order to insert it in the HTML. If you do as you suggest and simply have a MyClass LayoutData property every ViewModel uses to store that data, you are still going to need an interface that every ViewModel implements. Otherwise _Layout has no way to reference it without ViewData hacks. Making every ViewModel reimplement an interface of several properties will violate DRY. There is no mixing responsibilities - the view-specific data from your model belongs in the ViewModel. Ex: showing the title Edit user Erik's ProfileBergmann
Not true. Every layout page that does not have an explicit model type is dynamically typed to the type of the page being rendered. In other words, you need only have your LayoutData property contained in the ViewModel to access it from the layout, and pass it to your RenderAction method to render your navigation, or login/logout links or whatever you are doing in your layout. No ViewData or ViewBag is necessary. Of course, if you forget to include it, you will get a YSOD, but there are many things in MVC that are this way. So saying "there's no way" is not quite accurate.Kohn
You can argue that this is no better than ViewBag if you like, but in reality it's not much different from the way MVC itself operates, since it passes the model to the view via ViewData. It just hides this fact behind some properties. This is basically the same thing, you're passing the strongly typed object to your child action via a weakly typed conduit.Kohn
K
3

It's impossible to say, because we don't know what your model is and if there are any security issues present.

In general, the security issue is not in passing the model the view, but rather in receiving posts that are automatically bound to the data model. If you have sensitive information in there, it's possible for someone to craft a post that will change things you didn't intend.

See http://odetocode.com/blogs/scott/archive/2012/03/12/complete-guide-to-mass-assignment-in-asp-net-mvc.aspx

The bigger issue is, are they REALLY the same? And are you sure they will stay the same? Are you sure you won't need to make changes that require the view model to have different requirements from the data model somewhere down the line?

It's easier to do the work up front, and follow good design, than to try to fix it later when you need it.

Kohn answered 18/5, 2014 at 15:10 Comment(0)
M
3

Your are correct in your assumption that the extra layer adds complexity, redundancy, extra development time of mapping and synchronization. But this disadvantage is easily absorbed in a large complex application, specially in a team environment where different teams are assigned to different layers.

For small projects with a few developers (typically 1 in your case), it is okay to just use the same model in your UI. Being small, you can easily slap the extra layer when it does grow.

The point of the ViewModel is to support the separation of concern, which is the main selling point of MVC. This separation allows your presentation and domain to safely evolve independently. You can protect the domain from any changes in the presentation, and you can protect the presentation from any changes in the domain.

You might not notice this early on, but in the long run the domain and presentation is guaranteed to change, and your code needs to be ready when that happens. It is what you plan today that will keep you sane tomorrow.

Also, There are many ways to automate mapping from domain model to view model, tools like AutoMapper.

Manuel answered 18/5, 2014 at 15:33 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.