CODEWITHSUNDEEP
asp.net-mvcvalidation
Sailing Judo
Sailing Judo

Reputation: 11243

Which is more correct: using UpdateModel() or receiving a model as a parameter?

I've seen numerous examples of create actions in articles, books, and examples. It seem there are two prevalent styles.

 [AcceptVerbs(HttpVerbs.Post)]
 public ActionResult Create(FormCollection collection)
 {
     try
     {
         var contact = Contact.Create();
         UpdateModel<Contact>(contact);
         contact.Save();
         return RedirectToAction("Index");
     }
     catch (InvalidOperationException ex)
     {
         return View();
     }
 }

And...

 [AcceptVerbs(HttpVerbs.Post)]
 public ActionResult Create([Bind(Exclude="Id")]Contact contact)
 {
     try
     {
         contact.Save();  // ... assumes model does validation
         return RedirectToAction("Index");
     }
     catch (Exception ex)
     {
         // ... have to handle model exceptions and populate ModelState errors
         // ... either here or in the model's validation
         return View();
     }
 }

I've tried both methods and both have pluses and minuses, IMO.

For example, when using the FormCollection version I have to deal with the "Id" manually in my model binder as the Bind/Exclude doesn't work here. With the typed version of the method I don't get to use the model binder at all. I like using the model binder as it lets me populate the ModelState errors without having any knowledge of the ModelState in my model's validation code.

Any insights?

Update: I answered my own question, but I wont mark it as answered for a few days in case somebody has a better answer.

Upvotes: 7

Views: 2782

Answers (4)

Ross McNab
Ross McNab

Reputation: 11577

You should always accept the model as an action parameter, because it makes the method more testable.

It's far easier to pass in a model object from your unit test, than to attempt to mock out the whole HttpContext to provide data to be picked up by UpdateModel.

Even in the case where you want to update an existing entity model, you should still use this pattern by introducing a view model:

[HttpPost]
public ActionResult Edit(int id, EditContactViewModel viewModel)
{
    if (ModelState.IsValid)
    {
        Contact contact = _db.GetContact(id)

        contact.Name = viewModel.Name;

        _db.SaveChanges();

        return RedirectToAction("Index");
    }

    return View(viewModel);
}

Upvotes: 1

San
San

Reputation: 2346

Use UpdateModel when you want to update an already present model object, which you may get from database or you want to instantiate the model object in some specific way

eg:

 [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult EditEmployee(int id, FormCollection collection)
    {

try
     {

    Contact contact = repository.getContact(id);
    UpdateModel(contact, collection.ToValueProvider());
    repository.save();
    return RedirectToAction("Index");

}

    catch
    {
    //Handle 
    return View();
    }

}

If you dont have the above requirements then put it as action parameter.

Upvotes: 3

eu-ge-ne
eu-ge-ne

Reputation: 28153

There are no correct or incorrect styles. If DefaultModelBinder suits your needs - always use strongly-typed version. If you can not use DefaultModelBinder - choose between creating a custom ModelBinder or using FormCollection Action parameter.

Upvotes: 0

Sailing Judo
Sailing Judo

Reputation: 11243

Well, after some dinking around I discovered what should have been obvious: passing the model as the parameter merely causes the default model binder to get used behind the scenes. Since this was my biggest reason not to use the strongly-typed version, then I guess there is no reason not to now.

Also, my code should look like this:

 [AcceptVerbs(HttpVerbs.Post)]
 public ActionResult Create([Bind(Exclude="Id")]Contact contact)
 {
     if(!ModelState.IsValid)
         return View();

     contact.Save();
     return RedirectToAction("Index");
 }

Upvotes: 2

Related Questions