Fetching data within an ASP.NET MVC ViewModel class?

Question

For those that create ViewModels (for use by typed views) in ASP.NET MVC, do you prefer to fetch the data from a service/repository from within the ViewModel, or the controller classes?

For example, we started by having ViewModels essentially being DTOs and allowing our Controllers to fetch the data (grossly oversimplified example assumes that the user can only change employee name):

public class EmployeeViewModel
{
    public String Name; //posted back
    public int Num; //posted back
    public IEnumerable<Dependent> Dependents; //static
    public IEnumerable<Spouse> Spouses; //static
}

public class EmployeeController()
{
    ...
    public ActionResult Employee(int empNum)
    {
        Models.EmployeeViewModel model = new Models.EmployeeViewModel();
        model.Name = _empSvc.FetchEmployee(empNum).Name;
        model.Num = empNum;
        model.Dependents = _peopleSvc.FetchDependentsForView(empNum);
        model.Spouses = _peopleSvc.FetchDependentsForView(empNum);
        return View(model);
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Employee(Models.EmployeeViewModel model)
    {
        if (!_empSvc.ValidateAndSaveName(model.Num, model.Name))
        {
            model.Dependents = _peopleSvc.FetchDependentsForView(model.Num);
            model.Spouses = _peopleSvc.FetchDependentsForView(model.Num);
            return View(model);
        }
        this.RedirectToAction(c => c.Index());
    }
 }

This all seemed fine until we started creating large views (40+ fields) with many drop downs and such. Since the screens would have a GET and POST action (with POST returning a view if there was a validation error), we'd be duplicating code and making ViewModels larger than they probably should be.

I'm thinking the alternative would be to Fetch the data via the Service within the ViewModel. My concern is that we'd then have some data populated from the ViewModel and some from the Controller (e.g. in the example above, Name would be populated from the Controller since it is a posted value, while Dependents and Spouses would be populated via some type of GetStaticData() function in the ViewModel).

Thoughts?

Answer 1

I encountered the same issue. I started creating classes for each action when the code got too big for the action methods. Yes you will have some data retrieval in classes and some in the controller methods. The alternative is to have all the data retrieval in classes, but half the classes you won't really need, they will have been created for consistency sake or have all the data retrieval in the controller methods, but again, some of those methods will be too complex and needed to have been abstracted into classes... so pick your poison. I would rather have a little inconsistency and have the right solution for the job.

As for putting behavior into the ViewModel, I don't, the point of the ViewModel is to be a thin class for setting and extracting values from the View.

There have been cases where I've put conversion methods in the ViewModel. For instance I need to convert the ViewModel to the corresponding entity or I need to load the ViewModel with data from the Entity.

To answer your question, I prefer to retrieve data from with in the controller/action methods.

Typically with DropDowns, I create a dropdown service. DropDowns tend to be the same data that spans views. With the dropdowns in a service I can use them on other views and/or Cache them.

Depending on the layout, 40 plus fields could create a cluttered view. Depending the type of data, I would try to span that many fields across multiple views with some sort of tabbed or wizard interface.

Answer 2

There's more than that ;-) You can fetch in model binder or action filter. For the second option, check Jimmy Bogard's blog somewhere around here. I personally do it in model binders. I use ViewModel like this: http://stackoverflow.com/questions/1453641/my-custom-asp-net-mvc-entity-binding-is-it-a-good-solution. It is processed by my custom model binder:

public object BindModel(ControllerContext c, BindingContext b)
{
   var id = b.ValueProvider[b.ModelName]; // don't remember exact syntax
   var repository = ServiceLocator.GetInstance(GetRepositoryType(b.ModelType));
   var obj = repository.Get(id);
   if (obj == null)
     b.ModelState.AddModelError(b.ModelName, "Not found in database");
   return obj;
}

public ActionResult Action(EntityViewModel<Order> order)
{
   if (!ModelState.IsValid)
      ...;
}

You can also see an example of model binder doing repository access in S#arp Architecture.

As for static data in view models, I'm still exploring approaches. For example, you can have your view models remember the entities instead of lists, and

public class MyViewModel { public MyViewModel(Order order, IEmployeesSvc _svc) { }

  public IList<Employee> GetEmployeesList()
  {
      return _svc.GetEmployeesFor(order.Number);
  }

}

You decide how you inject _svc into ViewModel, but it's basically the same as you do for controller. Just beware that ViewModel is also created by MVC via parameterless constructor, so you either use ServiceLocator or extend MVC for ViewModel creation - for example, inside your custom model binder. Or you can use Jimmy Bogard's approach with AutoMapper which does also support IoC containers.

The common approach here is that whenever I see repetative code, I look to eliminate it. 100 controller actions doing domain-viewmodel marshalling plus repository lookup is a bad case. Single model binder doing it in generic way is a good one.

Answer 3

Here's another solution: http://www.lostechies.com/blogs/jimmy_bogard/archive/2009/06/29/how-we-do-mvc-view-models.aspx

Main points there:

1. Mapping is done by a mediator - in this case it is AutoMapper but it can be your own class (though more to code). This keeps both Domain and ViewModel concentrated on the domain/presentation logic. The mediator (mapper) will contain (mostly automatic) logic for mapping, including injected services.
2. Mapping is applied automatically, all you do is tell the action filter the source/destination types - very clean.
3. (Seems to be important for you) AutoMapper supports nested mappings/types, so you can have your ViewModel combined of several independent view models, so that your "screen DTO" is not messy.

Like in this model:

public class WholeViewModel
{
   public Part1ViewModel ModelPart1 { get; set; }
   public Part2ViewModel ModelPart2 { get; set; }
}

you re-use mappings for specific parts of your View, and you don't write any new line of code, since there're already mappings for the partial view models.

If you don't want AutoMapper, you have have IViewModelMapper interfaces, and then your IoC container will help your action filter to find appropriate

container.Resolve(typeof(IViewModelMapper<>).MakeGenericType(mysourcetype, mydesttype))

and it will also provide any required external services to that mapper (this is possible with AutoMapper, too). But of course AutoMapper can do recursions and anyway, why write additional AutoMapper ;-)

Answer 4

I wouldn't be fetching data from the database in your ViewModel. The ViewModel exists to promote separation of concerns (between your View and your Model). Tangling up persistance logic in there kind of defeats the purpose.

Luckily, the ASP.NET MVC framework gives us more integration points, specifically the ModelBinder.

I've got an implementation of a generic ModelBinder pulling information from the service layer at:-

http://www.iaingalloway.com/going-further-a-generic-servicelayer-modelbinder

It doesn't use a ViewModel, but that's easily fixed. It's by no means the only implementation. For a real-world project, you're probably better off with a less generic, more customised solution.

If you're diligent, your GET methods don't even need to know that the service layer exists.

The solution probably looks something like:-

Controller action method:-

public ActionResult Details(MyTypeIndexViewModel model)
{
  if( ModelState.IsValid )
  {
    return View(model);
  }
  else
  {
    // Handle the case where the ModelState is invalid
    // usually because they've requested MyType/Details/x
    // and there's no matching MyType in the repository
    // e.g. return RedirectToAction("Index")
  }
}

ModelBinder:-

public object BindModel
(
  ControllerContext controllerContext,
  BindingContext bindingContext
)
{
  // Get the Primary Key from the requestValueProvider.
  // e.g. bindingContext.ValueProvider["id"]
  int id = ...;

  // Get an instance of your service layer via your
  // favourite dependancy injection framework.
  // Or grab the controller's copy e.g.
  // (controllerContext.Controller as MyController).Service
  IMyTypeService service = ...;

  MyType myType = service.GetMyTypeById(id)

  if (myType == null)
  {
    // handle the case where the PK has no matching MyType in the repository
    // e.g. bindingContext.ModelState.AddModelError(...)
  }


  MyTypeIndexViewModel model = new MyTypeIndexViewModel(myType);

  // If you've got more repository calls to make
  // (e.g. populating extra fields on the model)
  // you can do that here.

  return model;
}

ViewModel:-

public class MyTypeIndexViewModel
{
  public MyTypeIndexViewModel(MyType source)
  {
    // Bind all the properties of the ViewModel in here, or better
    // inherit from e.g. MyTypeViewModel, bind all the properties
    // shared between views in there and chain up base(source)
  }
}

Build your service layer, and register your ModelBinder as normal.

Answer 5

Consider passing your services into the custom ViewModel on its constructor (ala Dependency Injection). That removes the model population code from your controller and allows it to focus on controlling the logical flow of the application. Custom ViewModels are an ideal place to abstract the preparation of things like SelectLists that your droplists will depend on.

Lots of code in the controller for things like retrieving data isn't considered a best practice. The controller's primary responsibility is to "control" the flow of the application.

Answer 6

Submitting this one late... Bounty is almost over. But...

Another mapper to look at is Automapper: http://www.codeplex.com/AutoMapper

And overview on how to use it: http://www.lostechies.com/blogs/jimmy%5Fbogard/archive/2009/01/22/automapper-the-object-object-mapper.aspx

I really like it's syntax.

// place this somewhere in your globals, or base controller constructor
Mapper.CreateMap<Employee, EmployeeViewModel>();

Now, in your controller, I would use multiple viewmodels. This enforces DRY by allowing you to reuse those viewmodels elsewhere in your application. I would not bind them all to 1 viewmodel. I would refactor to something like:

public class EmployeeController()
{
  private IEmployeeService _empSvc;
  private ISpouseService _peopleSvc;

  public EmployeeController(
      IEmployeeService empSvc, ISpouseService peopleSvc)
  {
    // D.I. hard at work! Auto-wiring up our services.  :)
    _empSvc = empSvc;
    _peopleSvc = peopleSvc;

    // setup all ViewModels here that the controller would use
    Mapper.CreateMap<Employee, EmployeeViewModel>();
    Mapper.CreateMap<Spouse, SpouseViewModel>();
  }

  public ActionResult Employee(int empNum)
  {
    // really should have some validation here that reaches into the domain
    //

    var employeeViewModel = 
        Mapper.Map<Employee, EmployeeViewModel>(
          _empSvc.FetchEmployee(empNum)
        );

    var spouseViewModel =
        Mapper.Map<Spouses, SpousesViewModel>(
          _peopleSvc.FetchSpouseByEmployeeID(empNum)
        );

    employeeViewModel.SpouseViewModel = spouseViewModel;

    return View(employeeViewModel);    
  }

  [AcceptVerbs(HttpVerbs.Post)]
  public ActionResult Employee(int id, FormCollection values)    
  {
    try
    {
      // always post to an ID, which is the employeeID
      var employee = _empSvc.FetchEmployee(id);

      // and bind using the built-in UpdateModel helpers.
      // this will throw an exception if someone is posting something
      // they shouldn't be posting. :)
      UpdateModel(employee);

      // save employee here

      this.RedirectToAction(c => c.Index());
    }
    catch
    {
      // check your domain model for any errors.
      // check for any other type of exception.  
      // fail back to the employee screen
      RedirectToAction(c => c.Employee(id));
    }
  } 
}

I generally try to stay away from saving multiple entities on a controller action. Instead, I would refactor the employee domain object to have AddSpouse() and SaveSpouse() methods, that would take an object of Spouse. This concept is known as AggregateRoots, controlling all dependancies from the root - which is the Employee() object. But, that is just me.