MVC - Controller with multiple select lists

Question

Are there any good ways to keep my controllers simpler when they have models that depend on a lot of select lists? I try to keep most of my controller actions as simple as possible (hopefully no more than 10 or so lines) but on pages that require a lot of dropdowns my actions usually exceed this:

public class Model
{
    public IEnumerable<SelectListItem> AllLocations { get; set; }
    public IEnumerable<SelectListItem> TopLocations { get; set; }
    public IEnumerable<SelectListItem> AllTemplates { get; set; }
    public IEnumerable<SelectListItem> TopTemplates { get; set; }
    // ...
}

[HttpGet]
public ActionResult Index(int id)
{
    var domain = Repository.Get(id);
    var model = Mapper.Map<Domain, ViewModel>(item);

    // any way to abstract this type of code?
    model.AllLocations = new SelectList(repository.GetAllLocations(), "Value", "Text");
    model.TopLocations = new SelectList(repository.GetTopLocations(), "Value", "Text");
    model.AllTemplates = new SelectList(repository.GetAllTemplates(), "Value", "Text");
    model.TopTemplates = new SelectList(repository.GetTopTemplates(), "Value", "Text");
    // etc. etc.

    return View(model);
}

[HttpPost]
public ActionResult Index(ViewModel model)
{
    // any way to abstract this type of code?
    model.AllLocations = new SelectList(repository.GetAllLocations(), "Value", "Text");
    model.TopLocations = new SelectList(repository.GetTopLocations(), "Value", "Text");
    model.AllTemplates = new SelectList(repository.GetAllTemplates(), "Value", "Text");
    model.TopTemplates = new SelectList(repository.GetTopTemplates(), "Value", "Text");
    // etc. etc.

    return View(model);
}

In general if you want to reduce controller code you could get the get action down to one line if you used model binding to convert Id to domain object and then use a custom action result to map and enrich model. Think a lot of that is covered in the book MVC 3 In Action — GraemeMiller, Feb 05 '12 at 10:42

score 3 · Accepted Answer · edited May 23 '17 at 11:48

As you say keeping controller actions small is great. As Jimmy Bogard says put your controllers on a diet!

I use an IModelEnricher combined with Automapper. I return an entity etc using a specific ActionResult that then automaps my entity to a ViewModel and enriches with data required for select lists (and any additional data required). This method keeps your code DRY and controllers thin like a super model :-)! Also keeping the select list data as part of your ViewModel keeps your controller, model, and view responsibilities clear.

Defining a ViewModel ernicher means that anywhere that ViewModel is used it can use the same enricher to get its properties. So you can return the ViewModel in multiple places and it will just get populated with the correct data.

In my case this looks something like this in the controller:

public virtual ActionResult Edit(int id)
{
    return AutoMappedEnrichedView<PersonEditModel>(_personRepository.Find(id));
}

[HttpPost]
public virtual ActionResult Edit(PersonEditModel person)
{
     if (ModelState.IsValid){
            //This is simplified (probably don't use Automapper to go VM-->Entity)
            var insertPerson = Mapper.Map<PersonEditModel , Person>(person);
            _personRepository.InsertOrUpdate(insertPerson);
            _requirementRepository.Save();
            return RedirectToAction(Actions.Index());
      }
     return EnrichedView(person);
 }

This sort of ViewModel:

public class PersonEditModel
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }
    public int FavouriteTeam { get; set; }
    public IEnumerable<SelectListItem> Teams= new List<SelectListItem>();
}

With this sort of Enricher:

public  class PersonEditModelEnricher :
IModelEnricher<PersonEditModel>
{
    private readonly ISelectListService _selectListService;

    public PersonEditModelEnricher(ISelectListService selectListService)
    {
        _selectListService = selectListService;
    }

    public PersonEditModelEnrich(PersonEditModel model)
    {
        model.Teams = new SelectList(_selectListService.AllTeams(), "Value", "Text")
        return model;
    }
}

One other option is to decorate the ViewModel with attributes that define how the data is located to populate the select list. Like:

  public class PersonEditModel
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public int Age { get; set; }
        public int FavouriteTeam { get; set; }
        [LoadSelectListData("Teams")]
        public IEnumerable<SelectListItem> Teams= new List<SelectListItem>();
    }

Now you can decorate an appropriate method in your select service with an attribute like:

   [ProvideSelectData("Teams")]
   public IEnumerable Teams()
   {
        return _teamRepository.All.ToSelectList(a => a.Name, a => a.TeamId);
   }

Then for simple models with no complex enrichment just the generic enrichment process can handle it. If you want to do anything more complex you can define an enricher and it will be used if it exists.

See this question. Also this blog post and this. Also this question on Automapper forum

score 2 · Answer 2 · answered Feb 04 '12 at 14:38

2

You can setup a helper class, put every select list in a static method.Then,in view you can get every select list by htmlhelper. Controller will clear. At the same time, other view can use these select list too.

eg:

public class SelectHelper
{
     public static List<SelectListItem> AllLocations()
     {
         //TODO repository.GetAllLocations()
     }
     public static List<SelectListItem> TopLocations()
     {
         //TODO repository.GetTopLocations()
     }
     ...
}

view code: @Html.DropDownList("selectname", SelectHelper.AllLocations())

answered Feb 04 '12 at 14:38

javan

74
4

I'd prefer the ViewModel to contain all data that is required to construct the view. Rather than having the view be responsible for getting data. – GraemeMiller Feb 04 '12 at 16:28
ViewModel is a good way, Sometimes I often let ViewModel simple, just a DTO.Next,I will do it in ViewModel. If many ViewModels has that select list, I think encapsulate a method is necessary. – javan Feb 04 '12 at 16:44
I just don't think the view should be getting data independently of the controller. Controller should act between view and data. However it obviously depends on app size and complexity. – GraemeMiller Feb 04 '12 at 16:55

score 1 · Answer 3 · answered Feb 03 '12 at 22:20

Sure, just refactor it to a method, like so:

public class Model
{
    public IEnumerable<SelectListItem> AllLocations { get; set; }
    public IEnumerable<SelectListItem> TopLocations { get; set; }
    public IEnumerable<SelectListItem> AllTemplates { get; set; }
    public IEnumerable<SelectListItem> TopTemplates { get; set; }
    // ...
}

[HttpGet]
public ActionResult Index(int id)
{
    var domain = Repository.Get(id);
    var model = Mapper.Map<Domain, ViewModel>(item);
    InitializeSelectLists(model);

    return View(model);
}

[HttpPost]
public ActionResult Index(ViewModel model)
{
    InitializeSelectLists(model);
    View(model);
}


private void InitializeSelectLists(Model model)
{
    model.AllLocations = new SelectList(repository.GetAllLocations(), "Value", "Text");
    model.TopLocations = new SelectList(repository.GetTopLocations(), "Value", "Text");
    model.AllTemplates = new SelectList(repository.GetAllTemplates(), "Value", "Text");
    model.TopTemplates = new SelectList(repository.GetTopTemplates(), "Value", "Text");
    // etc. etc.
}

Or you can even do it in a constructor for your model or a facade service if you want to.

MVC - Controller with multiple select lists

3 Answers3

Linked