30

I have a Service class with a method called GetProducts(). That encapsulates business logic and calls the repository to get a list of products.

My MVC view wants to show that list of products as an MVC SelectList. Where is the correct place for that logic to go. I seem to have 3 options:

  1. Model

    The Model should expose a property called ProductSelectList. When the getter of this property is called by the View, the Model should call Service.GetProducts() and convert the result to a SelectList before passing it on.

    Plausible argument: The Model should make calls to business logic and the repository. The View should merely render predetermined data. The Controller should not be involved, save for passing contextual data to the Model.

  2. View

    The View should contain code that calls Service.GetProducts() directly and converts the result to a SelectList inline.

    Plausible argument: The View should call for this data directly as it is specifically for use on the View. There is no need to involve the Model or Controller, as we are calling an abstracted Service method anyway, so anything else just adds extra overhead.

  3. Controller

    The Controller should make the call to Service.GetProducts(), convert the results to a SelectList and pass it through to the Model, which should contain a simple ProductSelectList property. The View will access this property for rendering.

    Plausible argument: The Controller knows which parameters to provide to the Service method, so it should make the call. The Model should be a simple placeholder for data, filled by the Controller. The View's job is to simply render the data from the Model.

I have a feeling that the correct answer is Model, but the other two make some reasonable points. Perhaps I've muddied the waters by already having a Service class that's separate to the Model?

Would anybody care to share their opinion? Is this just a matter of taste?

9 Answers 9

16

I personally subscribe to the logic of Number 3, allowing the controller to populate the Model (or View Model as is sometimes differentiated).

  • I have my views dumb and only displaying data.
  • I have my View Models store the information that the View will need, occasionally exposing 'get only' properties that format other properties into a nicer format. If my model needs access to my services, then I feel I'm doing something wrong.
  • The controllers arrange and gather all the information together (but do no actual work, that is left for the services.

In your example, I would have my controller action similar to:

public ActionResult Index()
{
    IndexViewModel viewModel = new IndexViewModel();
    viewModel.ProductSelectList = new SelectList(Service.GetProducts(), "Value", "Name");
    return View(viewModel);
}

and my view model similar to:

public class IndexViewModel()
{
   public SelectList ProductSelectList { get; set; }
   public int ProductID { get; set; }
}

With the appropriate part of the view looking like:

@Html.DropDownListFor(x => x.ProductID, Model.ProductSelectList);

This way I'm content that I know where to look if there is an issue with anything and everything has a very specific place.

However, there is no correct way as seems always to be the case with these things. Stephen Walther has a good blog series on MVC tips. In one he talks about the View Model emphasis and although not a SelectList he populates, the SelectList is still data in much the same way his list of products is.

2
  • A really good answer. Seems to run a little counter to what @AJ01 presents - what do you think about that? Could it be that there is no definitive right answer for MVC? This fat model / thin model argument seems more like a religious issue :) Commented Aug 10, 2011 at 21:58
  • It very much is a religious debate I think. I think the right answer would be to simply be consistent with your approach whatever it is. :)
    – Amadiere
    Commented Aug 11, 2011 at 12:11
9

You're right that there are a number of ways to handle this, and that's even before considering variations like MVP, MVVM, et cetera. Since you're asking about ASP.Net MVC in particular, I will defer to Microsoft:

An MVC model contains all of your application logic that is not contained in a view or a controller. The model should contain all of your application business logic, validation logic, and database access logic. For example, if you are using the Microsoft Entity Framework to access your database, then you would create your Entity Framework classes (your .edmx file) in the Models folder.

A view should contain only logic related to generating the user interface. A controller should only contain the bare minimum of logic required to return the right view or redirect the user to another action (flow control). Everything else should be contained in the model.

In general, you should strive for fat models and skinny controllers. Your controller methods should contain only a few lines of code. If a controller action gets too fat, then you should consider moving the logic out to a new class in the Models folder.

Source

I would say your call belongs in the Model.

0
8

In a classic MVC architecture your Model shouldn't be much more than a container for your view data hence the reason it's often called a ViewModel. A ViewModel is different from the Entity Model(s) that your service layer manages.

Your controller is then responsible for populating your ViewModel from the entity model(s) returned by your service layer.

Due to convenience some developers will use their service layer entities directly in their ViewModels but long term that can lead to headaches. One way around that is to use a tool such as AutoMapper to automate the shuffling of data to and from your ViewModel and entity models.

Here's what a controller might look like. Notice that data such as the SSN does not get exposed to the view since there is a mapping from your Entity Models to your View Model.

public class Customer : IEntity
{
  public string CustomerID { get; set; }
  public string SSN { get; set; }
  public string FirstName { get; set; }
  public string LastName { get; set; }    
  public Address Address { get; set; }
}

public class CustomerEditViewModel
{
  public string FirstName { get; set; }
  public string LastName { get; set; }
  public string Address1 { get; set; }
  public string Address2 { get; set; }
  public string Country { get; set; }
  public string City { get; set; }
  public string State { get; set; }
  public string Zip { get; set; }
  public string PhoneNumber { get; set; }
}

public class CustomerController
{
  [AcceptVerbs (HttpVerbs.Get)]
  public ActionResult Edit ()
  {
    Customer customer = _customerService.GetCustomer (User.Identity.Name);

    var model = new CustomerEditViewModel ()
    {
      FirstName = customer.FirstName,
      LastName = customer.LastName,
      Address1 = customer.Address.Address1,
      Address2 = customer.Address.Address2,
      Country = customer.Address.Country,
      City = customer.Address.City,
      State = customer.Address.State,
      Zip = customer.Address.Zip,
      PhoneNumber = customer.Address.PhoneNumber,
    };

    return View (model);
  }
}
3
  • 1
    Is the following summary correct then? The Controller calls the Model for data that it then uses to populate a ViewModel which it then passes to the View. Commented Aug 10, 2011 at 21:51
  • 1
    Not exactly. The Controller calls the Service Layer which returns Entity Models. The Controller then creates and populates a View Model using the Entity Model data. Once the View Model is complete the Controller pass it on to the View.
    – Todd Smith
    Commented Aug 10, 2011 at 23:38
  • 1
    Shouldn't the FirstName = customer.FirstName (etc) fields be better off in a CustomerEditViewModel constructor? Or is there a good reason I'm not aware of?
    – Flater
    Commented Jul 8, 2013 at 9:19
3

One thing to keep in mind is that the SelectList class is specific to MVC only. So in my opinion it shouldn't be included in any business logic, and model classes fall into that category. Therefore your select list should be a part of a view model class instead.

This is the way it works in my projects:

  • Controller method is called
  • Controller uses repository (business logic, in other words) to get model data
  • Controller converts the model data if necessary and creates a view model object
  • Controller passes the view model to the view
  • The view displays the data in the view model with limited logic to show or hide things, etc
2
  • The point that SelectList is a Presentation-level issue is a good one. But wouldn't that suggest a solution where the Model contains the list of product entities, and the View is responsible for converting them to a SelectList? Or are you saying it's okay for a ViewModel to address presentation-level concerns. Either way, your bullet points suggest that the Controller should do the work of getting the Product entities in the first place; is that right? Commented Aug 10, 2011 at 21:23
  • Yes, the controller should do the work of converting the model object(s) into the view model. The view shouldn't do any work that isn't related to displaying the data. In other words, it can iterate over loops and show or hide things, but nothing else. In my opinion a view model is directly related to your presentation, so it's okay to include things specific to MVC. Commented Aug 11, 2011 at 17:12
1

I'd go with option 3. In general, I'll construct my MVC apps such that the controller makes a call to the service to return a model (or collection of models) which are then passed to the view.

I generally keep my models very thin. They are a flattened representation of the data with validation attributes and that's it. I use a service (or model builder) layer to construct the models and do business logic on them. Some folks embed that into the model, but I find that makes for a messy project.

You definitely don't want the view making any calls to your services.

Update...
I'm assuming that this SelectList is your model. If instead it's a part of your model, then you're right, you should put it in your model. I generally don't like to make it a method call, though. I'd have a property on my model:

public SelectList Products { get; set; }

And have my service or model builder class actually populate it. I don't usually have any data-oriented methods on my models.

1

I'm going with option 1.

Models are the place to make calls to business logic, et cetera.

View - Should display only what the ViewModel already has been populated with.

Controller - the job of the Controller is to direct the traffic coming in (from Web requests) to the Logic that is responsible for handling the request. Hence the term 'controller'.

There are always exceptions to these, but the best place (structurally) is the Model.

1

I had this problem when I started into MVC and came up with this solution.

The controller talks to a Service Layer. The service layer contains my Domain models and does all the processing for request from the Controllers. The service layer also returns ViewModels to satisfy requests from the controller.

The service layer calls a repository and gets the entities it will need to build the ViweModels. I often use Automapper to populate the ViewModel or collections within the view model.

So, my view models contain all that is needed by the View, and the Controller is doing nothing but handling request and forwarding them to the appropriate service handler.

I don't see a problem with having view specific items like SelectLists in the view Model either.

1

None of the above.

In my web layer I basically just have html and javascript views. The model shouldn't leak into the view and neither should the services.

I also have an Infrastructure layer which binds the services and model to the views. In this layer there are ViewModels, which are classes that represent what will be displayed on the screen, Mappers, which do the work getting data from services/model and mapping it to the view model, and Tasks, which perform tasks such as Saving, Updating and Deleting data.

It is possible to put a lot of this infrastructure in the Controllers, similar to the example Todd Smith has given above, but I find for anything other than trivial views the Controller becomes littered with code to load data and populate view models. I prefer a dedicated single responsibility mapper class for each view model. Then my controller will look something like

public class CustomerController
{
  [AcceptVerbs (HttpVerbs.Get)]
  public ActionResult Edit (int id)
  {
    return View (CustomerEditMapper.Map(id));
  }

  [AcceptVerbs (HttpVerbs.Post)]
  public ActionResult Save(CustomerEditViewModel model)
  {
    var errors = CustomerEditUpdatorCommand.Execute(model);
    ModelState.AddErrors(errors);
    return View ();
  }

}
1

I'm torn between option 1 and option 3. I've ruled option 2 out completely as to me that's polluting the view with procedure calls not just presentation layer work.

Personally I would do it in the model and the getter would call the Service layer but I also subscribe to the belief that the model should only contain the information the view needs to render the page, by not fully containing the data in the model at the time you pass it to the view you are breaking this.

Another option here though would be to avoid tightly coupling the view and model by putting a Dictionary of the Products into the view through a Service Call then using the view to transform the Dictionary to a SelectList but this also gives you the ability to just output the information as well.

I think this boils down to a preference as to where you are happy having your logic.

1
  • 'ello :) It's a good point, regarding whether the View should be entirely ignorant of the rest of the application, only receiving primitives and simple collections. People are saying that the Model shouldn't leak into the View, but surely the approach of passing Models to Views necessitates a leak? I believe this is what ViewModels are for, but I think for a newbie the word Model is over-used and confusing. Commented Aug 11, 2011 at 8:23

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