Replacing elements in XElement breaks foreach loop

Question

I have string

<div class="TextP">
   <span class="bold" style="font-weight: bold;">bold</span> text 
   <span class="bold" style="font-weight: bold;">italic</span> text 
   <span class="bold" style="font-weight: bold;">underlined</span> text
</div>

which I parse to XElement object and than I need to replace the formating spans with other elements. So I have written this code

//el is the root div
foreach (XElement el in e.Elements())
        {
            switch (el.Name.ToString().ToLower())
            {
                //The method is more complex, but only this part doesnt work, therfore this only case
                case "span":
                    if (el.Attribute("class") != null)
                    {
                        switch (el.Attribute("class").Value)
                        {
                            case "underline" :
                                el.ReplaceWith(XElement.Parse("<U>" + el.Value + "</U>"));
                                break;
                            case "bold":
                                el.ReplaceWith(XElement.Parse("<B>" + el.Value + "</B>"));
                                break;
                            case "italic":
                                el.ReplaceWith(XElement.Parse("<I>" + el.Value + "</I>"));
                                break;
                        }
                    }
                    break;
            }
        }

The problem is that when I replace the first span the foreach loop breaks and the two other spans remain unreplaced. I think It's because the .Elements() collection changes, but I can't figure out, how should I change my code.

p.s.w.g · Accepted Answer · 2013-09-11T18:49:27.040

4

Generally you can't make changes to a collection as you're iterating through it. One way to get around this is to make a copy of your collection and iterate through that:

foreach (XElement el in e.Elements().ToArray()) // or ToList
{
    // ...
}

This will find all the child elements of e at the beginning of the loop and store them in a different collection (using the Linq ToArray / ToList method). This way, the elements collection can be freely modified inside the loop.

edited Sep 11 '13 at 18:49

answered Sep 11 '13 at 18:39

p.s.w.g

146,324
30
291
331

2

Personally I prefer `ToList` - I generally prefer lists over arrays, and I *think* in the case of LINQ to Objects, it's slightly more efficient too. – Jon Skeet Sep 11 '13 at 18:42
Nice solution, thanks! And one more question, is there any quick elegant way to instead of `el.Value` get `innerXml` because the formating spans can be nested of course and I need to use `e.Descendants()` instead – m3div0 Sep 11 '13 at 18:55
@JonSkeet It looks like both use an identical strategy--dynamically sized arrays starting at size 4 and doubling each time--but indeed, `ToList` saves one array copy because the final array is not re-sized back down before you get an enumerator. I learned something new today, thanks :) – p.s.w.g Sep 11 '13 at 18:57
1

@david See [Best way to get InnerXml of an XElement?](http://stackoverflow.com/questions/3793/best-way-to-get-innerxml-of-an-xelement) – p.s.w.g Sep 11 '13 at 19:05
@p.s.w.g I think the good alternative for ToList/ToArray is Reverse. – Hamlet Hakobyan Sep 11 '13 at 19:05
@p.s.w.g: Yes, it's the lack of the final array copy which I was referring to, but didn't want to go into too much detail ;) – Jon Skeet Sep 11 '13 at 19:15

Replacing elements in XElement breaks foreach loop

1 Answers1