passing a selectlist to the view based on database models from viewmodel is a MVC anti-pattern?

Question

We need to select a model reference from a view. To do this we need to add a dropdownlistfor. this is based on a selectlist containing database models.

passing selectlists by viewbag

this solves our problem, but we do not like using ViewBag.

public ActionResult Create()
    {
        ViewBag.SomeModels = new SelectList(context.SomeModel, "id", "modelDescription");

        return View();
    }

Our solution

We added a context to a viewmodel to do the same, but this is MVC anti-pattern.
below is a snippet of our code.

public class SomeModelViewModel : SomeModel
{
    private SomeContext context = new SomeContext ();

    public SelectList getSomeOtherModels()
    {
        return new SelectList(context.SomeOtherModel, "id", "modelDescription");
    }
}

Does anyone have suggestions for a clean way to solve this problem?

Answer 1

I think there are a few things that can be fixed here, but overall I don't think this is automatically an anti-pattern. It could just be executed a little better.

Caveat: I don't practice what I preach nearly as often as I should.

What I tend to do in my own projects, and the standard where I work, is for the view model to expose an IEnumerable<SelectListItem> (actually, when we're lazy, we put this enumerable in the ViewBag like you have but we're trying to get away from that.) Rather than giving the context to your view model, just give the items to the view model when it's constructed and have it generate the SelectListItems from them. AFAIK, that's the controller's job.

public class SomeViewModel
{
    public IEnumerable<SelectListItem> ListItems { get; private set; }

    public SomeViewModel(IEnumerable<SomeModel> items)
    {
        ListItems = items
            .Select(i => new SelectListItem()
                {
                    Text = i.Name,
                    Value = i.Id,
                    Selected = i.IsSelected // or whatever
                })
            .ToList();
    }
}

public class SomeController
{
    public ActionResult Index()
    {
        var viewModel = new SomeViewModel(_context.SomeModels);

        return View(viewModel);
    }
}

Again, I can't promise this is the more "correct" way to do it, but I've done it and seen it done on at least four different projects, and it's worked well for us so far.

P.S. Your ViewModel probably shouldn't inherit from your Model. I've never actually seen that done before, but some cursory Googling shows that it's not really recommended. You should have the exact properties you need in the view copied into your view model (I hear there are tools for his), and use a tool like AutoMapper to copy values back and forth when necessary.