Being on holidays gives me a lot of time to go through MVC questions on StackOverflow. And a lot of questions come from misusing of ViewBag and ViewData. A lot of people leave out Model from MVC and just slap all their data into ViewBag. And this is bad.

I can understand why people are lazy: [sarcasm] creating an extra class can be very problematic, takes a lot of keystrokes [/sarcasm]. What people don’t realise is that ViewBag make them work harder.

ViewBag is a dynamic entity and it can contain any property it likes with any type it likes. So every time you do in controller something like this:

    public ActionResult SomeViewBagAction()
    {
        ViewBag.SomeText = "blah";
        return View();
    }

You need to work at least 2 times harder. In a View you need to cast from ViewBag to String and then you need to lookup the exact spelling of the property you’ve just assigned because Visual Studio auto-complete does not work on dynamic objects. Plus your view code looks UUUGGGLY:

@Html.TextBox("NameOfTheTextbox", (String)ViewBag.SomeValue)

See that cast up there ^^^? That is what I mean by “ugly”. It must not be in a view. I might allow casts in controller or any other piece of server-side code, but not in a view. And not only this looks ugly and does not read easily, this is very error-prone. Are you sure that property of a ViewBag contains a String? Could it be some other object that you accidently slapped into a ViewBag? And you will only find this only during execution of the view. Or your customers will find that out for you.

See how the problem this can be created:

    public ActionResult SomeViewBagAction()
    {
        ViewBag.SomeText = tagRepository.GetTag("fail");
        return View();
    }

This will not trigger any kind of compile time alerts, but will pretty much fail on a run-time. Why? Because you are making yourself work hard by using a ViewBag. Because you must be sure that tagRepository.GetTag("fail") returns a String, not something else, like object of type Tag or System.Data.Entity.Infrastructure.DbQuery<Tag>. And to be sure of that you’ll need to look up the implementation (or interface) of tagRepository – and that is extra work. On top of what you already done extra.

USE A MODEL!

Don’t be lazy, create a model for every view you use. The architecture is called MVC – Model-View-Contorller. Not Bag-View-Controller. Don’t skip the first letter – Model.

And creating a model is not difficult, only a few extra keystrokes at the start of controller action, but much less effort afterwards. And model must be plain simple – only properties. Nothing else, no methods or functions.

This is much more sane implementation:

public class TagController : Controller
{
    public ActionResult SomeClassAction()
    {
        var model = new TagViewModel();
        // presuming tagRepository.GetTag(String key) returns object Tag with property Name on it
        model.Name = tagRepository.GetTag("fail").Name; 
        return View(model);
    }
}

public class TagViewModel
{
    public String Name { get; set; }
    public int NumberOfArticles { get; set; }
}

If you are trying to assign a DbQuery<Tag> or Tag to a String, you’ll get a compiler error, not run-time error.

After modification of a controller your View will be like this:

@model MyMvcApplication.Controllers.TagViewModel

<h2>title</h2>

@Html.TextBoxFor(m => m.Name)

No casting and run-time exceptions. If you use Resharper – it will pick up the error if you are trying to use incorrect property of the model in a view.

Refactoring with ViewModel is easier

Another thing why I don’t like ViewBag. If you use a model and trying to find out where a particular property is used in views – that’s easy: right click on a property -> Find Usages will show you all the places where you assign or consume a property. This is very handy in refactoring, when you try to decide if a property is used somewhere or not. For the case with a ViewBag – nothing will help you for this.

ViewBag can be overwritten everywhere

To finish off, you can get a funny bug where you get something not expected in your ViewBag property. You’ll spend a lot of time tracing the pipeline, where this ViewBag value is re-assigned. Because you can’t just see where ViewBag.SomeValue is re-assigned.

See this example again:

    public ActionResult VeryBuggyAction()
    {
        ViewBag.SomeValue = "This should be simple!";
        return View();
    }

And a view looks like this:

 <h2>@ViewBag.SomeValue</h2>

Do you expect to see “This should be simple!” on the page?

WRONG! Because some other developer for some reason put a global filter into the application that overwrites a ViewBag in a nasty way:

public class ViewBagRewritingFilter : ActionFilterAttribute
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        // this erases your earlier value
        filterContext.Controller.ViewBag.SomeValue = "Now, try catch me!";

        // worst case that can happen to ViewBag is get null assigned, but this is more probable than above
        //filterContext.Controller.ViewBag = null;

        base.OnActionExecuted(filterContext);
    }
}

public static void RegisterGlobalFilters(GlobalFilterCollection filters)
{
    filters.Add(new ViewBagRewritingFilter());
}

This can take some time to trace down because there is no connection between controller action and the filter. And all these trouble for laziness.

Don’t be lazy, people, create models for your views!