Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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?

share|improve this question
2  
In such cases i'm adding my selectlist to my model and pass it to my view, but stil wondering if there is a better solution + –  Jeroen Feb 14 '13 at 9:34
    
I personally don't think its an anti-pattern at all, the model is exclusively for the view so inclusion of view constructs is IMO not a bad thing –  Luke McGregor Feb 14 '13 at 10:06
    
In my opnion this is an anti-pattern, because I have a context in the model layer and contexts are bound to controllers. –  annemartijn Feb 14 '13 at 13:20
    
Why do you need to pass the SelectList? Can't you have the items for the select list in one Property, and the selected value in another? –  Jeff Vanzella Feb 14 '13 at 16:58
add comment

migrated from stackoverflow.com Feb 14 '13 at 16:40

This question came from our site for professional and enthusiast programmers.

1 Answer

up vote 4 down vote accepted

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.

share|improve this answer
1  
I love the idea that the ViewModel requires the list so make it to the constructor. Never thought of that and actually I liked it A LOT. Thanks. –  Bart Feb 25 '13 at 23:32
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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