Null collections/arrays from MVC model binding

I’m firmly in the camp of the Framework Design Guidelines guidance on collection properties and return values:

DO NOT return null values from collection properties or from methods returning collections. Return an empty collection or an empty array instead

It just makes sense, right? Well, that’s not the default behavior in MVC. If you have a form something like:

public class OrderForm {
  public string FirstName { get; set; }
  public string LastName { get; set; }
  public LineItem[] LineItems { get; set; }
}

You might expect to always have some value for that LineItems property, even if no LineItems were posted up. You would be disappointed. Instead, if there are no values found for anything in that collection, the property is null. That’s OK for primitives – I can expect default values or Nullable types to be null. For collections, I don’t want to worry about checking for null on every single item. Iterators and enumerable extension methods all blow up with nulls.

Instead, I’d rather these default to empty values. This is fairly easily done by augmenting the default model binder:

    public class BetterDefaultModelBinder : DefaultModelBinder
    {
        protected override object CreateModel(ControllerContext controllerContext, ModelBindingContext bindingContext, Type modelType)
        {
            var model = base.CreateModel(controllerContext, bindingContext, modelType);

            if (model == null || model is IEnumerable)
                return model;

            foreach (var property in modelType.GetProperties(BindingFlags.Public | BindingFlags.Instance))
            {
                object value = property.GetValue(model);
                if (value != null)
                    continue;

                if (property.PropertyType.IsArray)
                {
                    value = Array.CreateInstance(property.PropertyType.GetElementType(), 0);
                    property.SetValue(model, value);
                }
                else if (property.PropertyType.IsGenericType)
                {
                    Type typeToCreate;
                    Type genericTypeDefinition = property.PropertyType.GetGenericTypeDefinition();
                    if (genericTypeDefinition == typeof(IDictionary<,>))
                    {
                        typeToCreate = typeof(Dictionary<,>).MakeGenericType(property.PropertyType.GetGenericArguments());
                    }
                    else if (genericTypeDefinition == typeof (IEnumerable<>) ||
                             genericTypeDefinition == typeof (ICollection<>) ||
                             genericTypeDefinition == typeof (IList<>))
                    {
                        typeToCreate = typeof(List<>).MakeGenericType(property.PropertyType.GetGenericArguments());
                    }
                    else
                    {
                        continue;
                    }

                    value = Activator.CreateInstance(typeToCreate);
                    property.SetValue(model, value);
                }
            }

            return model;
        }
    }

And configuring it to be the default model binder in our global.asax:

ModelBinders.Binders.DefaultBinder = new BetterDefaultModelBinder();

The model binder will overwrite these properties later on if those collections do have items. In fact, it’s one of the design goals of AutoMapper – never allow null collections. With this code, we ensure that we never have to worry about null collections ever again on our view models, either generating them with AutoMapper or binding them with MVC.

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Jimmy Bogard

I'm a technical architect with Headspring in Austin, TX. I focus on DDD, distributed systems, and any other acronym-centric design/architecture/methodology. I created AutoMapper and am a co-author of the ASP.NET MVC in Action books.
This entry was posted in ASP.NET MVC. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Rémi BOURGAREL

    i’d prefer to create an extension method EmptyIfNull() because it’ll be more explicit instead of hidden behind a class referenced outside of the current execution scope.

    • jbogard

      You know, that was my first approach and one I’ve used in the past. But then I couldn’t be sure when I needed to do it, or would have to “remember” to do that before I iterated a collection. I can see that being explicit sometimes is better though.

    • Chuck Bryan

      Would the Model Binder approach fall under “Tell, Don’t Ask”?

  • Chuck

    As much as I have read your posts, I am only starting to appreciate Model Binding inside MVC. Whether it is this example, or a binder that Trims Strings, it is nice to be able to put expectations on your model. Thanks for the Article.

  • Pingback: Null collections/arrays from MVC model binding ...

  • Jonathan Allen

    Are you required to define your models that way?

    According to the .NET framework design guidelines you are not supposed to expose setters on collections. Instead you would have…

    _LineItems = new List();
    public List LineItems { get {return _LineItems; } }

    • jbogard

      Design guidelines for messages/serialized types are a little different. A lot of those tools expect setters everywhere.

      • http://www.Marisic.Net/ dotnetchris

        I find it absolute insanity that the default MVC model binder given the scenario

        _LineItems = new List();
        public List LineItems { get {return _LineItems; } }

        Will modify _LineItems.

        If i have no setter, and i receive data posted under LineItems, my model should have nothing in _LineItems.

  • regisbsb

    Jimmy, may I borrow your unit tests for the BetterDefaultModelBinder??

    • jbogard

      Yes, you may borrow my invisible tests.

      • regisbsb

        I mean, the unit tests for that, that you have in the project you applied the BetterModelBinder.

        • jbogard

          uhhh

          • regisbsb

            Haahuahaua LOL!
            Thanks for the binders anyway that saved the day here.

          • http://www.Marisic.Net/ dotnetchris

            One simply goes and cries in the corner

  • bob

    Would it be worthwhile to add “if (property.GetSetMethod() == null) continue;” to the beginning of the foreach loop? I would think there would be no point in checking for null collections and arrays if there isn’t a public setter that the model binder can use.

  • http://www.Marisic.Net/ dotnetchris

    Jimmy, isn’t this just easier to handle by doing

    public class OrderForm {
    public OrderForm()
    {
    if(LineItems == null) LineItems = new []{}
    }

    • http://www.Marisic.Net/ dotnetchris

      I really wish there was an operator like LineItems ??= new[]{}, set if null coalesce operator

    • jbogard

      Well, not sure if it’s easier, but it is more explicit (and would work for testing too).

    • http://www.joshka.net/ Joshua McKinney

      Is there a need to null check in the constructor?
      I am inclined to always initialise values for things that shouldn’t be null. View models are no different in this regard. Hence, with this in mind, I see the binder changes as unnecessary. This simplifies unit testing as well as your test becomes:

      var orderForm = new OrderForm();
      controller.SubmitOrder(orderForm);

      instead of:
      var orderForm = new OrderForm { LineItems = new[] {} };

      • http://www.Marisic.Net/ dotnetchris

        “Is there a need to null check in the constructor?” I don’t know.

        These classes are always data transfer objects and could be serialized in any shape or form. When deserialization happens I have no idea about order of operations. I never want my real data to ever end up with a blank collection overwriting a real collection so I always choose to be safe than sorry.

    • http://blog.deltacode.be/ David De Sloovere

      Copy pasting code instead of refactoring to be reusable is also easier. But after a few paste operations, you know it’s wrong, don’t you?

      • http://www.Marisic.Net/ dotnetchris

        Not sure what you’re trying to get at here

  • Cal

    Do you have a link to the Framework Design Guidelines that you are referring to?

  • http://www.ebog.me/ ebog

    Thanks for sharing about your codes. It is really necessary and useful for our users as

  • khalidabuhakmeh

    So model binding in MVC works in an interesting way. If the property is not found in the ValueProviders, then it is ignored. So you can actually set a default on any property, in the constructor of your ViewModel works just fine. I think most people think it is a straight up serialization/deserialization process, but it isn’t.

    • Marcel Popescu

      Ah… thank you; I wasn’t aware of this and it’s a great insight. Initializing the collection inside the ViewModel constructor makes a lot of sense.

  • http://devtools.korzh.com/ devtools.korzh.com

    Thank you. It was interesting and helpful

  • regisbsb

    There is a problem when you have a array type you get “Collection is read-only.” error on BindProperty.