Unintended change of variable

Question

When I do change List of Groups in method EditSelectList, it also affect ViewBag.Groups. Why is it happening? I assigned ViewBag earlier. It should not affect ViewBag.Groups and var tournamentsGroups

I have this code in controller:

[AllowAnonymous]
public ActionResult Details2(int id)
{
    var tournamentsContext = new TournamentsContext();
    var tournament = tournamentsContext.Tournament.Single(t => t.Id == id);

    var groupsContext = new GroupsContext();
    var tournamentsGroups = groupsContext.Groups.Where(g => g.IdTournament == tournament.Id && g.SexMale == true).ToList();
    ViewBag.Groups = tournamentsGroups;

    ViewBag.groupId = new SelectList(EditSelectList(tournamentsGroups), "Id", "Belt");

    ViewBag.tournamentId = id;

    ViewBag.InTournament = CurrentUserInTournament(tournament.Id, currentUser);

    return View(tournament);
}

private List<Groups> EditSelectList(List<Groups> groups)
{
    foreach (var item in groups)
    {
        item.Belt = item.Belt + " - " + item.WeightKg; // for dropdownlist
    }

    return groups;
}

and View

@model TournamentApp.Models.Tournament

@foreach (var item in ViewBag.Groups)
{
    if ("white".Equals(@item.Belt) || true)
    {
        <h4>@item.Belt</h4>
    }
}

// ... some another code for dropdownlist

so result is:

white - 65
white - 75
white - 85
blue - 75

but it should be just:

white
white
white
blue

`ToList()` returns a `List`. That you then *reference* from several locations (`ViewBag.Groups`, `tournamentsGroups`, and when you pass it to `EditSelectList`). But there's still only one `List` object. — Damien_The_Unbeliever, Dec 23 '15 at 11:25

score 1 · Accepted Answer · edited May 23 '17 at 11:44

Because the list in ViewBag.Groups still contains the same objects as the original list tournamentsGroups. The reference to the list is copied, its contents isn't changed at all. There is still just one list.

You need to copy the entire list and its items to prevent your subsequent actions to change the list you assign.

If you only use one property (Belt), you might want to create a new list of the strings, which is much easier than cloning every object. If you require to clone every since object, take a look at Deep cloning objects.

Unintended change of variable

1 Answers1