MVC where should the logic go the Controller or the View Model

Question

I have an MVC app where I'm wanting to display a dropdownlist with info from the database.

The dropdown will display info from database Cars using the table Make which is the make of the car.

So in my view I will have something like:

@model VectorCheck.ViewModels.CarsViewModel
...

@Html.DropDownListFor(modelItem => Model.MakeId, Model.Makes)
...

So somehow I need to get the view model the list of makes.

So I might have some logic to go with this say only cars that are colour Red.

var redCars = _unitOfWork.Cars(x => x.Colour == "Red");

So my question is where is the best practise to put the logic for this query. Should it go in the viewModel or controller.

The way I see it I have two Options.

Option 1: The controller.

public ActionResult Edit(int id)
        {
            var car = _unitOfWork.CarRepository.Get(id);

            var carMakes = _unitOfWork.CarMakeRepository.Where(x => x.Colour == "Red").Select(u => new SelectListItem { Value = u.CarMakeId.ToString(), Text = u.Name });

            return View("Edit", new InsertUpdateCarViewModel(car, carMakes));
        }

ViewModel

public Car Car { get; set; }
public IEnumerable<SelectListItem> CarMakes { get; set; }

InsertUpdateCarViewModel(Car car, IEnumerable<SelectListItem> carMakes)
{
   Car= car;
   CarMakes = carMakes;

}

So in this example I get the carMakes in the controller and give them to the viewModel which is simply a container.

Opon 2: The viewModel

public ActionResult Edit(int id)
        {
            var car = _unitOfWork.CarRepository.Get(id);

            return View("Edit", new InsertUpdateCarViewModel(car));
        }

ViewModel

public Car Car { get; set; }
public IEnumerable<SelectListItem> CarMakes { get; set; }

InsertUpdateCarViewModel(Car car)
{
   Car= car;

   CarMakes = _unitOfWork.CarMakeRepository.Where(x => x.Colour == "Red").Select(u => new SelectListItem { Value = u.CarMakeId.ToString(), Text = u.Name });

}

So in this option I'm putting the logic to get the correct carmakes in the view model. It is more than a container.

So what I'm wanting to know is which of these ways is the correct way of doing this?

Answer 1

In the controller. The ViewModel should not be aware of the unit of work you are using. Also, the view model in this case would be a lot more reusable if it didn't have to rely on the logic x => x.Colour == "Red". Even though this could be moved to the arguments, in general, I believe your models (and therefor views) would be much more reusable taking care of that in the controller.

Answer 2

As already answered, it is the controller. To make it more memorable for you, I would put it this way. Do not let your view talk to the database directly. View asks/talks to Controller ONLY. Then obviously it makes sense for the view to send a request to the controller which forwards it to the database. Hope this helps for future!