using the ViewModel to store the mapping logic was not such a good idea because there was repetition and SRP violation, but now with the ValueInjecter I have clean ViewModels and dry mapping code

That's the old stuff, don't use it:I made a ViewModel pattern for editing stuff in asp.net mvc
this pattern is useful when you have to make a form for editing an entity and you have to put on the form some drop-downs for the user to choose some values

public class OrganisationBadViewModel
{
//paramterless constructor required, cuz we are gonna get an OrganisationViewModel object from the form in the post save method
public OrganisationViewModel() : this(new Organisation()) {}
public OrganisationViewModel(Organisation o)
{
Organisation = o;
Country = new SelectList(LookupFacade.Country.GetAll(), "ID", "Description", CountryKey);
}
//that's the Type for whom i create the viewmodel
public Organisation Organisation { get; set; }
...
}

4 Answers
4

The terminology. ViewModel is this case is a simple view data that is populated and later consumed by controller. View knows nothing about controller as ASP.NET MVC infrastructure is responsible for selecting controllers and appropriate actions. Controller handles user interaction. I think it looks more like Passive View than ViewModel (I assume that by ViewModel you mean Model-View-ViewModel pattern).

The details. The controller that populates view data is not supposed to know the details of how the view is implemented. However OrganisationViewModel.Country discloses unnecessary details (SelectListItem is pure view implementation detail). Thus making controller dependent on view implementation details. I think it should be changed to avoid it. Consider using some object that will hold the data for a country.

This looks very similar to the recommended practice in the Wrox Professional ASP.NET MVC book, the first chapter of which is available for free from the above URL.

Starting on page 100 they have a section on ViewData and ViewModels.

When a Controller class decides to render an HTML response back to a client, it is responsible for explicitly passing to the view template all of the data needed to render the response. View templates should never perform any data retrieval or application logic – and should instead limit themselves to only have rendering code that is driven off of the model/data passed to it by the controller.

[...]

When using [the "ViewModel"] pattern we create strongly-typed classes that are optimized for our specific view scenarios, and which expose properties for the dynamic values/content needed by our view templates. Our controller classes can then populate and pass these view-optimized classes to our view template to use. This enables type-safety, compile-time checking, and editor intellisense within view templates.

Taken from "Chapter 1 "Nerd Dinner" from Professional ASP.NET MVC 1.0 written by Rob Connery et al published by Wrox". The original is available at http://tinyurl.com/aspnetmvc

We started doing this, but our controllers started becoming monstrous (since our ViewModels were not necessarily mapped 1:1 to our database). To alleviate this, we created Mapper classes that create the ViewModel and then map back to data bound for the database. The controller then just calls the Mapper class methods. Seems to work well.