Stop using [Bind]

There's a lot to like about ASP.NET MVC, but some parts are just atrocious. The Bind attribute is one of those atrocious parts. For the seemingly few who are not aware of this attribute, Bind provides a way to include/exclude certain properties from participating in model binding on post. For example, let's say you had an entity like:

public class Article
{
    [Key]
    public int Id { get; set; }

    public string Title { get; set; }

    private DateTime? createdDate;
    public DateTime CreatedDate
    {
        get { return createdDate ?? DateTime.UtcNow; }
        set { createdDate = value; }
    }
}

Here, you would never want a user to post a value for either Id (since it's an auto-incremented primary key) or CreatedDate (since it's implemented to be set to the current date and time on creation). You can, then ensure this via Bind:

public ActionResult Create([Bind("Title")]Article article)

With that, even if values were posted for Id or CreatedDate, they would simply be discarded. Alternatively, you could do [Bind(Exclude = "Id,CreatedDate")] to achieve the same effect.

What's wrong with it?

You may find yourself asking what's so wrong with doing this? Admittedly, it seems pretty straight-forward. However, there's three big issues:

1. You must include/exclude all relevant properties

Our example above was not so complex, but imagine a much larger class with many more properties. If you use the include syntax, you must list out every single last property you want to be included which could be quite a few. You could potentially switch to the exclude syntax if there's less properties you want excluded than included, but even then, there still could be quite a few. Simply, it's just a big eye-sore. Here's a slightly more complex version of Article:

public ActionResult Create([Bind("Title,Slug,Author,PublishDate,Headline,Content")]Article article)

And, even that is only six properties. Imagine 10 or 20. I just gets insane.

2. It's not DRY (Don't Repeat Yourself)

Each time you post this entity, you must add the same Bind signature again. In other words, if you have multiple actions that accept this entity, you end up with this same list of properties repeated over and over again. There's no way to factor it out in some abstract way.

3. It's not strongly-typed

This is the big one, as it builds on the two previous reasons. Since all the properties are just strings, there's no indication of whether they are valid or not, or even if they are inclusive. Imagine a scenario where you have all these Binds slewn about in your codebase and then you decide to add a new property to Article. If you used the include syntax, then you must find every single instance where you applied Bind and add the property to the existing list. What happens if you forget one? Simply, nothing. You won't get an exception or even a warning. The data will simply just be discarded, silently. Before you know it, you find yourself banging your head against a wall trying to desperately figure out why part of your model isn't posting, before you finally realize after hours of fruitless wasted energy and effort that you forgot to include it in the Bind declaration.

If you think using the exclude syntax is any better, consider the possible security holes you may open up if you add a new property that the user shouldn't be able to alter, but forget to add it to the list of excluded properties. All of a sudden users can mangle data on your entities, and you may not even notice until it's far too late.

Or, forget adding properties. What if you just rename a property? Or remove a property? It's just a maintenance nightmare, no matter how you cut it.

What should I do?

Instead, always use view models. A view model is just a class that is specified for the needs of a view or set of views. It may represent one of your entity classes or it may have no relationship to your database at all. Taking our article example from above:

public class ArticleViewModel
{
    public string Title { get; set; }
}

...

public ActionResult Create(ArticleViewModel model)

Since we don't want users to be able to edit Id or CreatedDate, we don't even include them here. If values are posted for anything other than Title, they are simply discarded, because the view model class itself does not contain any other properties. Now, it's the class, rather than Bind that's making the call, so you're 100% strongly-typed, and you're DRY because you only need to modify the view model class, itself, in just one place to make changes.

Also, you have an extra level of control with a view model. Since the view model, itself, is not persisted, you are responsible for mapping values from it to your entity class. Therefore, you can make decisions about what should and should not be mapped. For example, if you wanted to present the value of CreatedDate without allowing it to be modified, you would add that property to your view model. That then technically allows a user to post a value for CreatedDate, but, you can simply not map that value over. As a result, it doesn't matter if the user posts it or not, it will never effect anything on the entity unless you explicitly allow it.

There's countless reasons to use view models in MVC, and this is just one more. Do yourself a favor, and don't use Bind. Period. It's just bad design. Using view models, instead, also has the added benefit of encouraging another best practice: never posting to an entity class directly, which can cause a whole world of issues on its own.

comments powered by Disqus