5

I am trying to work out the best way of using a viewmodel in the case of creating a new object.

I have a very simple view model that contains a contact object and a select list of companies.

private ICompanyService _Service;
public SelectList ContactCompanyList { get; private set; }
public Contact contact { get; private set; }

public ContactCompanyViewModel(Contact _Contact)
{
    _Service = new CompanyService();
    contact = _Contact;
    ContactCompanyList = GetCompanyList();
}

private SelectList GetCompanyList()
{
    IEnumerable<Company> _CompanyList = _Service.GetAll();
    return new SelectList(_CompanyList, "id", "name");       
}

I then have contact controller that uses this viewmodel and enable me to select a related company for my contact.

[Authorize]
public ActionResult Create()
{                       
    return View(new ContactCompanyViewModel(new Contact()));
}

My issue is with the create method on the controller.

[Authorize]
[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Create(Contact _Contact)
{
    try
    {
        _Service.Save(_Contact);
        return RedirectToAction("Index");
    }
    catch
    {
       return View();
    }
}

The problem is that the view returns an empty contact object, but! the company id is populated, this is because the dropdown list explicitly declares its field name.

 @Html.DropDownList("parent_company_id",Model.ContactCompanyList)

The standard html form fields pass the objects values back in the format of contact.forename when using the HTML.EditorFor helper...

@Html.EditorFor(model => model.contact.forename)

I can access them if I use a FormCollection as my create action method paremeter and then explicitly search for contact.value but I cannot use a Contact object as a parameter to keep my code nice and clean and not have to build a new contact object each time.

I tried passing the actual view model object back as a parameter but that simply blows up with a constructor error (Which is confusing seeing as the view is bound to the view model not the contact object).

Is there a way that I can define the name of the Html.EditFor field so that the value maps correctly back to the contact object when passed back to the create action method on my controller? Or Have I made some FUBAR mistake somewhere (that is the most likely explanation seeing as this is a learning exercise!).

Mahmoud Gamal
  • 78,257
  • 17
  • 139
  • 164
David Abraham
  • 197
  • 1
  • 11

1 Answers1

15

Your view model seems wrong. View models should not reference any services. View models should not reference any domain models. View models should have parameterless constructors so that they could be used as POST action parameters.

So here's a more realistic view model for your scenario:

public class ContactCompanyViewModel
{
    public string SelectedCompanyId { get; set; }
    public IEnumerable<SelectListItem> CompanyList { get; set; }

    ... other properties that the view requires
}

and then you could have a GET action that will prepare and populate this view model:

public ActionResult Create()
{
    var model = new ContactCompanyViewModel();
    model.CompanyList = _Service.GetAll().ToList().Select(x => new SelectListItem
    {
        Value = x.id.ToString(),
        Text = x.name
    });
    return View(model);
}

and a POST action:

[HttpPost]
public ActionResult Create(ContactCompanyViewModel model)
{
    try
    {
        // TODO: to avoid this manual mapping you could use a mapper tool
        // such as AutoMapper
        var contact = new Contact
        {
            ... map the contact domain model properties from the view model
        };
        _Service.Save(contact);
        return RedirectToAction("Index");
    }
    catch 
    {
        model.CompanyList = _Service.GetAll().ToList().Select(x => new SelectListItem
        {
            Value = x.id.ToString(),
            Text = x.name
        });
        return View(model);
    }
}

and now in your view you work with your view model:

@model ContactCompanyViewModel
@using (Html.BeginForm())
{
    @Html.DropDownListFor(x => x.SelectedCompanyId, Model.CompanyList)

    ... other input fields for other properties

    <button type="submit">Create</button>
}
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
  • Thanks, that solved the problem by enabling me to map the viewmodel to the create action method, the only issue I then had was that the drop down value was not bound, I solved this by changing the drop down name to **@Html.DropDownList("contact.parent_company_id",Model.ContactCompanyList).** My only question is about why you are mapping manually? You have the model which contains a populated contact object, there is nothing to map? – David Abraham Mar 04 '12 at 10:30
  • 1
    @DavidAbraham, I mapped manually to illustrate the process. In a real application I use AutoMapper: http://automapper.org/ to do this job. – Darin Dimitrov Mar 04 '12 at 10:32
  • But again there is no need for Automapper either, in my case I have now passed back a viewmodel object that contains a contact domain object that is now populated fully, there is nothing to map - where does auto mapper come into it? – David Abraham Mar 04 '12 at 10:35
  • 3
    @DavidAbraham, the whole point of my answer is that a view model should not contain domain models. – Darin Dimitrov Mar 04 '12 at 10:38
  • I just read your answer again, that makes sense but it introduces a whole lot of code in the controller which I solved by simply using the domain model in the view model, by doing that I can reduce 1000s of lines of code in an application. I wont keep pestering as I now have it working well this way! Thank you for taking the time to answer. – David Abraham Mar 04 '12 at 10:45
  • @DavidAbraham, this is exactly what AutoMapper is designed for. – Darin Dimitrov Mar 04 '12 at 10:46
  • I am not sure I agree! I think you answered my question with "View models should have parameterless constructors" - I shifted the select list population to the controller but even that feels wrong, its business logic being placed in the wrong place. My controller for a contact now has to populate dependent company information - that code feels like it should sit in the view model which is responsible for building the data required for the view. That being said, you clearly know more than me on the subject so I am sure I am going to find a problem with this at some point :) – David Abraham Mar 04 '12 at 10:50
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/8495/discussion-between-david-abraham-and-darin-dimitrov) – David Abraham Mar 04 '12 at 10:54
  • 1
    Using your domain model in the view makes your view dependant on the domain design. `Using view-models in views and mapping:` If your domain model needs to change the worst is to change the mappings in a single place. `Only using domain-models in views:` If your domain model changes you need to update all the views consuming your domain model. Sure your design works for you and will most likely never break. It's up to you at the end :) – Nope May 23 '12 at 14:27