Which Method of Model Binding Has Best Unit Test Semantics in ASP.NET MVC?

Question

Definitions

In ASP.NET MVC, there are two ways to do model binding in an action. Let's call these the "Bind arguments way" and the "UpdateModel way." Both of them do almost exactly the same thing, and they do it in almost exactly the same way:

    public ActionResult UpdateWithBindArguments(Foo model)
    {
        Repository.Update(model);
        // error handling removed
        return RedirectToAction(...)
    }

    public ActionResult UpdateWithUpdateModel()
    {
        Foo model; 
        UpdateModel(model); // part of MVC framework
        Repository.Update(model);
        // error handling removed
        return RedirectToAction(...)
    }

As I said, these do almost exactly the same thing. The first is maybe slightly more readable, but I can get over that.

Two Ways to Test

The important difference, I think, is how you unit test them:

    [TestMethod]
    public void TestUpdateWithBindArguments()
    {
       var model = new Foo() { PropertyName = "Bar" };
       var controller = new FooController();

       var result = controller.UpdateWithBindArguments(model);

       // assert
    }

    [TestMethod]
    public void TestUpdateWithUpdateModel()
    {
       var formData = new FormCollection() { { "PropertyName", "Bar" } };
       var controller = new FooController();
       controller.ValueProvider = formData.ToValueProvider();

       var result = controller.UpdateWithUpdateModel();

       // assert
    }

The first method builds a model using strong, static typing. The second constructs submitted user data with name/value pairs. I find the first method a little easier to read, but the second method much closer to what actually happens when the controller is invoked by a web site.

For reasons well outside the scope of this question, I've never been of the persuasion that one should build aspx pages using lambda expressions instead of strings for model binding. I'd be happy to have that discussion with you, but let's not do it here. For the purpose of this question, let's take it for granted that I will be using the built-in HtmlHelper methods which take strings instead of extensions to it which take lambda expressions. Therefore, the second method's use of name/value pairs has some value as an informal test against the "dynamic" nature of the aspx pages. Of course, it does not replace integration testing against the site.

The Question (Finally!)

I see advantages and disadvantages to both methods. My question is, is there a really strong argument in favor of one method which I am missing?

Edit I am looking for objective answers. I am looking for non-obvious reasons why one method is better than the other, not trying to take an opinion poll.

Answer 1

Hi Craig.

I'd be inclined to write it like so

public ActionResult Update(int id, string name)
{
  Person person = PersonRepository.GetByID(id);
  //Do any error handling
  person.Name = name;
  //Do any error handling
  PersonRepository.Update(person);
}

Then test like so (using RhinoMocks)

Person person = new Person();
var mockPersonRepository = MockRepository.GenerateMock<IPersonRepository>();
mockPersonRepository.Expect(x => x.GetByID(1)).Return(person);
mockPersonRepository.Expect(x => x.Update(person));

var controller = new MyController(mockPersonRepository);
controller.Update(1, "Hello");
Assert("Hello", person.Name);
mockPersonRepository.VerifyAllExpectations();

So my answer is, none of the above :-)

Answer 2

I would always opt for transparency to the context to keep noise out of tests, so the first method is better IMO.

Just curious what you like about magic strings over expressions for HTML generation. Is it their invisibility to the compiler, resistance to refactoring, or maybe you hate intellisense. :) Uh oh, I've done it now, started an off topic debate.

Answer 3

I like the first option of binding to a model object. This frees the controller from the details of the model and view. Therefore, I can alter the model and view as needed without affecting my controller. Test isolation also becomes easier because of this.

For complex domain models or views that are only concerned with a fraction of the domain, I create a view model that models the data in my views instead of the data in my domain. I then write mapping code or use an object mapper to map the view model to my domain model. This reduces the coupling between my view and my domain.

I will have to disagree with Peter Morris on passing individual parameters to the controller method. This looks good at first but quickly becomes a pain once you start working with large forms. Besides, this increase coupling between the controller and the view.

Answer 4

In terms of testing, it seems to me that passing in the object directly to the action method is more natural. There's no need to populate the ValueProviderDictionary.

The reason the other approach is needed is that you might need to control the instantiation of the object you're binding. The DefaultModelBinder simply looks for a default constructor and calls it.

But in some cases, you might need to create the object yourself before binding it to the form values. That's where UpdateModel comes into play.