ViewBag vs Model, in MVC.NET

Question

This is more of a generic architectural question:

I'm trying to decide if it's ok for my programmers to use "ViewBags" to pass data to views that already accept Models.

My personal preference is to avoid ViewBags and build Robust Models containing all the data the view requires:

Approach 1:

MODEL A: 
- List of Employees
- Nullable integer, indicating which item from the list is currently selected
- string firstName (empty if index is null)
- string lastname (empty if index is null)

Approach 2:

MODEL A: 
- List of Employees

ViewBag:
- ViewBag.Index (indicating which item from the list is currently selected)
- ViewBag.FirstName
- ViewBag.LastName

Can anyone think of an argument why Approach2 would be better than approach 1?

thanks for your input

Answer 1

In my opinion, you should never use ViewBag without a very, very good reason. Brad Thomas' answer points out a rare example of one of those good reasons.

Say you have a master layout (or a partial view) shared by an entire website and dozens of strongly-typed Views that each have their own Model. How do you pass data to the layout? Some strategies I've seen:

1. Add the layout properties to every model.

If you don't have many models or it's just one or two extra properties this could work. This can quickly become a maintenance nightmare.

2. Have all your models inherit from a class that holds the layout data.

I avoid creating a ModelBase class, but it can be necessary sometimes.

3. Create a model for the layout and a generic model base class.

I've seen MVC apps with several layouts that any view could utilize. If your models inherit from a base class, you may need to have a base class for each layout. To avoid duplication of effort, I would create a layout model for each of the layouts. Then something like this might work for you:

abstract class ModelBase<TLayout> {
    public TLayout Layout { get; set; }
}

class Model : ModelBase<LayoutModel2> { /* model stuff here */ } 

4. Put layout properties in ViewBag.

There are rare situations I can imagine where putting the layout info in the model isn't the right call. It seems pretty commonplace for people to use their domain models as their model, and you may not want to mix layout/view data with the business/domain data, for example.

---

If you decide to use ViewBag, avoid this:

ViewBag.Title = "My page"
ViewBag.UserID = 123
ViewBag.UserName = "admin"
ViewBag.UserDisplayName = "Administrator"

There are obvious potential problems, like using different capitalization in different places (e.g. UserId instead of UserID). Less obvious is that someone could accidentally set ViewBag.UserID to a string or Nullable<int>:

class SomeOtherClass {
    public string UserID { get; set; } // someone uses a string...
}

ViewBag.UserID = someOtherClassObj.UserID; // now you're in trouble.

So if you must use ViewBag, I'd recommend something like this:

ViewBag.LayoutModel = new LayoutModel { UserID = User.ID, UserName = User.Name };