ViewBag multiple SelectListItems

Question

Getting some very annoying behaviour from my MVC3 app. So I've got a model with 3 values:

[DisplayName("Airlines ")]
[StringLength(2, ErrorMessage = "Airline codes can only be 2 characters in length")]
public string AirlineCode1 { get; set; }

[DisplayName("Airlines")]
[StringLength(2, ErrorMessage = "Airline codes can only be 2 characters in length")]
public string AirlineCode2 { get; set; }

[DisplayName("Airlines")]
[StringLength(2, ErrorMessage = "Airline codes can only be 2 characters in length")]
public string AirlineCode3 { get; set; }

Now these are populated from a DropDowList so I'm popping the DropDownListItems in the ViewBag and rendering them in the view such:

@Html.LabelFor(l => l.AirlineCode1) <span>(select/enter code in box on right)</span>
<div id="airportCode1">
    @Html.DropDownListFor(d => d.AirlineCode, ViewBag.AirLines as List<SelectListItem>) <input type="text" maxlength="2" value="@Model.AirlineCode1" />
</div>
<div id="airportCode2" style="@Model.AirlineCode2Style">
     @Html.DropDownListFor(d => d.AirlineCode, ViewBag.AirLines2 as List<SelectListItem>) <input type="text" maxlength="2" value="@Model.AirlineCode2" />
</div>
<div id="airportCode3">
    @Html.DropDownListFor(d => d.AirlineCode3, ViewBag.AirLines3 as List<SelectListItem>) <input type="text" maxlength="2" value="@Model.AirlineCode3" />
</div>

So my controller looks like:

IEnumerable<SelectListItem> airLines = PopulateAirlines(user);
ViewBag.AirLines = airLines;
ViewBag.AirLines2 = airLines;
ViewBag.AirLines3 = airLines;

Now in some circumstances I want to prepoulate AirLineCode in the model. so I set the model value in the controller. This resulted in some odd behaviour. Suddenly all my DropDownLists contained the prepopulated value!

Checked the model, value only set in AirLineCode1. Checked the ViewBag, no selected SelectListItems. So I figured the ViewBag must be maintaining a reference. So I changed my code to:

ViewBag.AirLines = PopulateAirlines(user);
ViewBag.AirLines2 = PopulateAirlines(user);
ViewBag.AirLines3 = PopulateAirlines(user);

Boom, fixed. Problem is PopulateAirlines is an expensive process!

Problem is the ViewBag appears to be maintaing a reference between the 3 SelectListItem Lists. How do I stop it doing this and still only make one call to PopulateAirlines(user);?

I tried the below code and this completely blew up:

IEnumerable<SelectListItem> airLines = PopulateAirlines(user);
ViewBag.AirLines = airLines;
ViewBag.AirLines2 = airLines.Select(s => new SelectListItem() { Text = s.Text, Value = s.Value, Selected = s.Selected });
ViewBag.AirLines3 = airLines.Select(s => new SelectListItem() { Text = s.Text, Value = s.Value, Selected = s.Selected });

with the error:

There is no ViewData item of type 'IEnumerable' that has the key 'AirlineCode2'.

What??!

Get rid of ViewBag. It's not strongly typed and an ugly way to pass data around. Create a ViewModel with your various Airline info items inside of it, problem solved : ) — Adam Tuliper, Jan 02 '13 at 17:34
I never agree with this argument. I don't want this data back, it's just to populate the drop down lists, so it'e more efficient to use ViewBag as it prevents the lists being serialised in both directions. For me ViewBag is for read only create and forget data. — Liam, Jan 03 '13 at 08:51
No, it's not really more efficient. Its basically negligible and just because its in a view model one direction doesn't mean it has to be on the receiving view model, or even bound for that matter. On the flip side you have brittle implementations prone to refactoring errors, hidden behavior behind the scenes by using viewbag in the html helpers, and a generally recommended against practice in anything but the smallest web apps. Some go so far as saying never use it (I generally accept it for a std use like page title). See Darin Dmitrov's many posts on it here for some good refs — Adam Tuliper, Jan 03 '13 at 16:43
I've heard this argument several times now and while I understand it, for me it just doesn't quite ring true. I think that by putting these into my model or ViewModel all I'm doing is cluttering it up with a load of objects that I just don't want to be in there. If it was data that'd be one thing but this is simply a mechanism to render a drop down list effectivly! Anyway, I got it sorted but it did involve refactoring my PopulateAirlines method. — Liam, Jan 04 '13 at 11:37

Quinton Bernhardt · Accepted Answer · 2013-01-03T11:13:20.933

It may be because of in this code:

IEnumerable<SelectListItem> airLines = PopulateAirlines(user);
ViewBag.AirLines = airLines;
ViewBag.AirLines2 = airLines;
ViewBag.AirLines3 = airLines;

all the ViewBag properties are referring to the same list. A change to the list will therefore reflect when referencing any of the properties. When you call the PopulateAirlines() method you're creating 3 different lists.

You'll need make three separate lists but only create the first using the PopulateAirlines method and then clone it twice. That will not be as expensive.

ViewBag multiple SelectListItems

1 Answers1