MVC setting up Html.DropdownList on ModelState.IsValid = false

Question

This is something that has always puzzled me as to the best way round, while keeping maintainable code. The below code sets up a list of months and years for a payment gateway form, before assigning these to a variable of type List<SelectListItem>.

Intial Action

PayNowViewModel paymentGateway = new PayNowViewModel();
List<SelectListItem> paymentGatewayMonthsList = new List<SelectListItem>();
List<SelectListItem> paymentGatewayYearsList = new List<SelectListItem>();

for (int i = 1; i <= 12; i++)
{
    SelectListItem selectListItem = new SelectListItem();
    selectListItem.Value = i.ToString();
    selectListItem.Text = i.ToString("00");

    paymentGatewayMonthsList.Add(selectListItem);
}

int year = DateTime.Now.Year;
for (int i = year; i <= year + 10; i++)
{
    SelectListItem selectListItem = new SelectListItem();
    selectListItem.Value = i.ToString();
    selectListItem.Text = i.ToString("00");

    paymentGatewayYearsList.Add(selectListItem);
}

paymentGateway.ExpiryMonth = paymentGatewayMonthsList;
paymentGateway.ExpiryYear = paymentGatewayYearsList;

return View(paymentGateway);

It's a fair bit of code, and I find myself repeating this code, in similar formats to re-setup the dropdown lists options should the ModelState.IsValid be false and I want to return back to the view for the user to correct there mistakes.

HttpPost Action - Code

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult ConfirmPayment(PayNowViewModel paymentGatewayForm, FormCollection form)
{
    if (ModelState.IsValid)
    {
        // Post processing actions...
        return View();
    }
    else
    {
        for (int i = 1; i <= 12; i++)
        {
            SelectListItem selectListItem = new SelectListItem();
            selectListItem.Value = i.ToString();
            selectListItem.Text = i.ToString("00");

            paymentGatewayMonthsList.Add(selectListItem);
        }

        int year = DateTime.Now.Year;
        for (int i = year; i <= year + 10; i++)
        {
            SelectListItem selectListItem = new SelectListItem();
            selectListItem.Value = i.ToString();
            selectListItem.Text = i.ToString("00");

            paymentGatewayYearsList.Add(selectListItem);
        }

        form.ExpiryMonth = paymentGatewayMonthsList;
        form.ExpiryYear = paymentGatewayYearsList;

        return View("MakePayment", form);
    }
}

What's the best way to centralise this dropdown setup code so its only in one place? At present you'll see a large proportion (the for loops), is exactly repeated twice. A base controller with function? Or is it better to re-setup like the above?

Any advice appreciated! Mike.

Answer 1

Add a private method to your controller (the following code assumes your ExpiryMonth and ExpiryYear properties are IEnumerable<SelectListItem> which is all that the DropDownListFor() method requires)

private void ConfigureViewModel(PayNowViewModel model)
{
  model.ExpiryMonth = Enumerable.Range(1, 12).Select(m => new SelectListItem
  {
    Value = m.ToString(),
    Text = m.ToString("00")
  });
  model.ExpiryYear = Enumerable.Range(DateTime.Today.Year, 10).Select(y => new SelectListItem
  {
    Value = y.ToString(),
    Text = y.ToString("00")
  });
}

and then in the GET method

public ActionResult ConfirmPayment()
{
  PayNowViewModel model = new PayNowViewModel();
  ConfigureViewModel(model);
  return View(model);
}

and in the POST method

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult ConfirmPayment(PayNowViewModel model)
{
  if (!ModelState.IsValid)
  {
    ConfigureViewModel(model);
    return View(model);
  }
  .... // save and redirect (should not be returning the view here)
}