Unexpected Foreach behavior C#

Question

The foreach loop have an unexpected behavior when the condition is true in fact after all instruction inside if statement, the loop breaks. I tried to comment the if statement and all works fine (iterate all the elements into the ienumerable). Someone can explain me why?

var allRef = projDefinition
    .Element(msbuild + "Project")
    .Elements(msbuild + "ItemGroup")
    .Elements(msbuild + "COMReference")
    .Where(refElem => find.Any(f => refElem.FirstAttribute.Value.ToLower().Contains(f)))
    .Select(refElem => refElem);


foreach (var xElement in allRef)
{
    var name = xElement.FirstAttribute.Value;
    var dllPath = dllFiles.FirstOrDefault(dll => dll.ToLower().Contains(name.ToLower()));

    if (dllPath != null)
    {
        var dllName = dllPath.Substring(dllPath.LastIndexOf('\\') + 1, dllPath.LastIndexOf('.') - dllPath.LastIndexOf('\\') - 1);
        xElement.Remove();
        XElement elem = new XElement(msbuild + "Reference", new XAttribute("Include", dllName));
        elem.Add(new XElement(msbuild + "HintPath", HintPath.GetHintPath(dllPath)));
        elem.Add(new XElement(msbuild + "EmbedInteropTypes", "False"));
        projDefinition.Root.Elements(msbuild + "ItemGroup").ElementAt(0).Add(elem);
    }

}

projDefinition.Save(fullProjectPath);

any exceptions while debugging line-by-line through the if block? — scrat.squirrel, Jan 04 '17 at 17:08
do you mean `if (dllPath != null)` messes with foreach loop? if this is your actual code then I wont believe what you said without seeing it with my own eyes. — M.kazem Akhgary, Jan 04 '17 at 17:09
You are modifying the source sequence inside the `foreach`, and LINQ queries use deferred execution. Add `.ToList()` at the end of `allRef` query. — Ivan Stoev, Jan 04 '17 at 17:09
My bet is `xElement.Remove();` modyfying `allRef` while still iterating through it. — Zbigniew, Jan 04 '17 at 17:10
so why there are any exception that tell me this? and what's exactly happen? someone please explain me. I know that LINQ use deferred execution, but I can't understand why it breaks the loop and tell me no exception. — CRK, Jan 04 '17 at 17:11
@CRK When altering the source collection while in a foreach you risk confusing the iteration. In your case you are removing an item in the source which can make it believe that the collection is done. There is no exception because it doesn't realize that it did not iterate through the entire collection. — Caleb Haldane, Jan 04 '17 at 17:14
@CRK The LINQ methods check to see what the next item will be when you ask it for that next item, so if you remove a given item after you construct the query but before you ask for that item, then it won't be returned, because *at the time you asked for another item* it wasn't there. — Servy, Jan 04 '17 at 17:15
thank you for the explanation. So why it works with a list? It don't have to have the same behavior? — CRK, Jan 04 '17 at 17:15
You might want to look into `Path.GetFileNameWithoutExtension` to simplify getting the `dllName`. — juharr, Jan 04 '17 at 18:00

score 5 · Answer 1 · edited May 23 '17 at 12:33

You're looping over allRef while at the same time you're removing from it with xElement.Remove, which messes with loop iterator.

Basicaly foreach creates IEnumerator under the hood and uses it to move from current element to next. You're disturbing it by removing element from collection while still going over it.

It will not cause exception, but your loop will cause unexpected behaviour like not going over all elements.

You may want to read how does foreach works

Simple solution for this kind of problem is adding .ToList() (as @Ivan Stoev noted in comments). It works because it creates new list so foreach iterator is created to that new list instead of your allRef. Which means that you can safely remove elements from allRef.

score 2 · Answer 2 · answered Jan 04 '17 at 17:29

For some more detail on how this works specifically, an XDocument is essentially similar to 'linked list' of nodes. Each element has a reference to its sibling and to its parent.

The various queries (like Elements) all use iterator blocks and are lazily evaluated. You can follow this in the reference source.

Once you call Remove on the current loop variable, the element is removed from the document tree. The references it had to its parent and its sibling are now both null. When the iterator resumes on the call to MoveNext as part of your foreach loop, there's nowhere to go; iteration complete.

As explained in the comments & other answers, a simple call to ToList before iterating over that will resolve the issue as it will iterate through your query and cache it in a new list. Removing nodes from the document later cannot affect the content of your list.

Unexpected Foreach behavior C#

2 Answers2