Deleting a node in foreach loop

Question

Look at this csharp code, and see if you can tell why I need to exit the loop after I found and deleted an item from the list. The idea is to go through a node's list of neighbors, and see if a Node n exists there, then delete that neighbor:

    internal void RemoveDirected(Node n)
    {
        foreach (EdgeToNeighbor etn in this.Neighbors)
        {
            if (etn.Neighbor.Key == n.Key)
            {
                RemoveDirected(etn);
                break;
            }
        }
    }

    internal void RemoveDirected(EdgeToNeighbor e)
    {
        Neighbors.Remove(e);
    }

. . .

    // Removes EdgeToNeighbor instance from AdjacencyList
    protected internal virtual void Remove(EdgeToNeighbor e)
    {
        base.InnerList.Remove(e);
    }

Notice how I have a "break" after the RemoveDirected call in the first method. I've found that if I didn't exit after the RemoveDirected it would go on forever in the foreach loop. I suppose it must have something to do with the way foreach works. If you modify the list that foreach is working on, it gets confuse and loops forever.

Have you seen this type of thing, and what are other options to use rather than using break? Of course, I could place the node that I've found in a local variable, then break from the loop, and delete it outside of the loop. But I was thinking, may be there are better ways to do this in csharp.

Answer 1

When you iterate a .NET collection using its iterator, you must not modify that collection. If you do, you are asking for trouble.

You should defer the deletion instead of deleting right in the foreach loop. For example, you can collect everything you need to delete in a list, and then delete it outside of foreach.

var toDelete = this.Neighbors.Where(etn => etn.Neighbor.Key == n.Key).ToList();
foreach (var e in toDelete) {
    Neighbors.Remove(e);
}