As my last post suggested, there is a problem (or bug) in ASP.NET MVC RC1 when adding model errors with AddModelError(string key, string errorMessage) without calling SetModelValue(string key, ValueProviderResult result). Failing to use SetModelValue() to set a value will result in a NullReferenceException with some HtmlHelpers.

A reply on a thread at the asp.net forums by a member of the ASP.NET team:

If you have a custom binder that calls AddModelError() without calling SetModelValue(), the HTML helpers will sometimes throw a NullReferenceException when you try to use them.  The reason is that a call to AddModelError() without a call to SetModelValue() doesn't really mean anything.  Consider that model errors are due to the user submitting bad form data, and by using the HTML helpers you're implicitly telling us that you want them to redisplay the user's invalid submitted form data, but the binder never told us what the bad data was!  It only gave the helpers half the data necessary to render themselves properly; it told them that there was an error, but it didn't provide the expected old value.

Obviously it's not really desirable behavior for us to throw this exception.  We're looking into ways to address this, but choosing an answer isn't as trivial as it seems.  We could silently fall back to a blank value, we could throw a different exception, etc., but in any case we're going to confuse and irritate developers who wrote custom binders and wonder why the users' bad values aren't being propagated between requests.

I've three issues with this. First and foremost the way the AddModelError() and SetModelValue() are linked together, the AddModelError() method wont work if the other method is not invoked and vice versa smells bad to me. It's like setting up an object with a constructor that will create an object in a state where it still needs more data to be functional. My suggestion to fix this issue would be to add another argument to the AddModelError() method, containing the attempted value. This way it's not possible to "forget" to call the SetModelValue() method.

My second issue is the rename of the method from SetAttemptedValue() to SetModelValue(). I dont think the name of the method express it's intent very well. The SetAttemptedValue() is much more clear on what its used for.

The third issue I have is the arguments to the SetModelValue() method.

void SetModelValue(string key, ValueProviderResult result)

The second argument is a ValueProviderResult object. Which has the following constructor:

ValueProviderResult(object rawValue, string attemptedValue, CultureInfo culture)

My issue here is the first argument, object rawValue, what's this? Looking through the source code to ASP.NET MVC RC1 revealed that it's the value that gets converted to a string and then displayed by the HtmlHelpers. The string attemptedValue doesn't get used in that scenario, it only has a few usages in the ASP.NET MVC source code.

In the project I'm currently working on I've created an extension method to the ModelStateDictionary to ease the use of adding a model error. It looks like this:

  1: public static void AddModelError

  2:   (this ModelStateDictionary modelState, string key, string errorMessage, string attemptedValue)

  3: {

  4:   modelState.AddModelError(key, errorMessage);

  5:   modelState.SetModelValue(key, new ValueProviderResult(attemptedValue, attemptedValue, null));

  6: }