Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Altering object is affecting previous versions of object in a foreach loop

So, this one is pretty straight forward I think.

Here is my code:

Dictionary<int, IEnumerable<SelectListItem>> fileTypeListDict = new Dictionary<int, IEnumerable<SelectListItem>>();

foreach (PresentationFile pf in speakerAssignment.FKPresentation.PresentationFiles)
{
    IEnumerable<SelectListItem> fileTypes = Enum.GetValues(typeof(PresentationFileType))
                .Cast<PresentationFileType>().Select(x => new SelectListItem
        {
             Text = x.ToString(),
             Value = Convert.ToString((int)x),
             Selected = pf.Type == (int)x
        });

        fileTypeListDict.Add(pf.ID, fileTypes);
}

What is happening is that in the end the dictionary will have all the correct keys, but all of the values will be set to the fileTypes list created during the final iteration of the loop. I am sure this has something to do with the object being used as a reference, but have not seen this issue before in my time with C#. Anyone care to explain why this is happening and how I should resolve this issue?

Thanks!

like image 475
PFranchise Avatar asked Oct 29 '12 13:10

PFranchise


1 Answers

This is the infamous "foreach capture issue", and is "fixed" in C# 5 ("fixed" is a strong word, since it suggests it was a "bug" before: in reality - the specification is now changed to acknowledge this as a common cause of confusion). In both cases, the lambda captures the variable pf, not "the value of pf during this iteration" - but in C# before-5 the variable pf is technically scoped outside the loop (so there is only one of them, period), where-as in C# 5 and above the variable is scoped inside the loop (so there is a different variable, for capture purposes, per iteration).

In C# 4, just cheat:

foreach (PresentationFile tmp in speakerAssignment.FKPresentation.PresentationFiles)
{
    PresentationFile pf = tmp;
    //... as before

now pf is scoped inside the foreach, and it will work OK. Without that, there is only one pf - and since you've deferred execution until the end, the value of the single pf will be the last iteration.

An alternative fix would be: don't defer execution:

fileTypeListDict.Add(pf.ID, fileTypes.ToList()); // note the ToList

Now it is evaluated while the pf is "current", so will have the results you expected.

like image 110
Marc Gravell Avatar answered Sep 28 '22 00:09

Marc Gravell