C# foreach loop outer string not updating as normal

Question

I know that's it's better to write Xml documents with the proper class but I'm short in time and want a quick result.

I tried to write the xml file in a string then save it as an xml file. The result is not correct, because the string is getting only the last value of the foreach loop every time.

I searched the problem and found out that I need to initiate the string inside the foreach loop, but I didn't understand the concept and didn't know where exactly.

What should I modify in my code?

string siteMap = "<? xml version = \"1.0\" encoding = \"UTF-8\" ?>";

List<string> Niveau1Titles = getNiveau1Titles(aux1);
List<List<Couple>> Niveau2Titles = getNiveau2Titles(aux1);

int i = 0;
foreach (var n2 in Niveau2Titles)
{
    siteMap += "<Niveau_1 Title = \"" + Niveau1Titles[i] + "\" >";
    foreach (var n22 in n2)
    {
        siteMap += "<Niveau_2 Title = \"" + n22.Title + "\" Link = \"" + n22.Link + "\" >";
        List<Couple> Niveau3Titles = new List<Couple>();
        Niveau3Titles = getNiveau3Titles(n22.Link);
        foreach(var n3 in Niveau3Titles)
        {
            siteMap += "<Niveau_3 Title = \"" + n3.Title + "\" Link = \"" + n3.Link + "\" />";
        }
        siteMap += "</Niveau_2>";

    }
    siteMap += "</Niveau_1>";
    i++;
}


Console.WriteLine(siteMap);

`The result is not correct, because the string is getting only the last value of the foreach loop every time` There's 3 `foreach` in your code, which one is causing the problem? — The_Black_Smurf, Jul 13 '15 at 22:13
At the first glance there is nothing wrong with the code. It should work as expected. Can you show us the example which gives you unexpected result? Also, have you verified if functions getNiveauXTitles all give you correct data? — Andrei, Jul 13 '15 at 22:15
I does not see, where your code create single root element. P.S. I really does not understand, how not using proper XML API can save your time. — user4003407, Jul 13 '15 at 22:18
the inner one (the third foreach) is making the problem. When I used Console.WriteLine(), the results were correct. — Deep Blue, Jul 13 '15 at 22:23
@DeepBlue, Update your current and desired output ? And also include all `getNiveauTitles` functions + `NiveauXTitles` classes. We always need all the relevant information. — Orel Eraki, Jul 13 '15 at 22:24
Good lord! Consider using StringBuilder.Append(), instead of all the plussing. — Shar1er80, Jul 13 '15 at 22:29
@DeepBlue, that only adds to confusion. What do you mean "results were correct"? You only output result once, at the very end, so this means the entire result was correct. So what's the problem then? — Andrei, Jul 13 '15 at 22:29
@DeepBlue.. i see the following code siteMap += ""; This is at the beginning of first for loop. Is that intentional? This might cause it break if the number of items in the 2 lists are not equal. — AlwaysAProgrammer, Jul 13 '15 at 22:37
I added a Console.Write() after every "siteMap+=" and I got the correct result in output . My problem is that the string "siteMap" contains the same "Niveau3_Title" . Got it ? — Deep Blue, Jul 13 '15 at 22:46
You should really learn how to work with XML. It is never a good idea to use string-based APIs to manipulate XML - the rules are different. That said, what leads you to believe that all of your loops execute? Have you stepped through them with the debugger? Since your XML is not indented and has no line breaks, it is possible that you didn't read it correctly when you used Console.WriteLine. — John Saunders, Jul 14 '15 at 03:44

score 2 · Answer 1 · answered Jul 14 '15 at 04:02

It doesn't seem overly hard to invest 5 mins to ensure that this is generated using LINQ to XML:

var result =
    new XDocument(
        new XDeclaration("1.0", "UTF-8", "yes"),
        new XElement(
            "sitemap",
            Niveau2Titles.Zip(Niveau1Titles, (n2, n1) =>
                new XElement(
                    "Niveau_1",
                    new XAttribute("Title", n1),
                    n2.Select(n22 =>
                        new XElement(
                            "Niveau_2",
                            new XAttribute("Title", n22.Title),
                            new XAttribute("Link", n22.Link),
                            getNiveau3Titles(n22.Link).Select(n3 =>
                                new XElement(
                                    "Niveau_3",
                                    new XAttribute("Title", n3.Title),
                                    new XAttribute("Link", n3.Link)))))))));

jdweng · Accepted Answer · 2015-07-14T03:48:33.567

I think the code is working correctly. You have multiple root elements in your xml. You need to add a root. Also you have an extra space in the xml identification line. Try code below.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            string aux1 = "";
            //remove extra space
            string siteMap = "<?xml version = \"1.0\" encoding = \"UTF-8\" ?>";
            siteMap += "<Root>";

            List<string> Niveau1Titles = getNiveau1Titles(aux1);
            List<List<Couple>> Niveau2Titles = getNiveau2Titles(aux1);

            int i = 0;
            foreach (var n2 in Niveau2Titles)
            {
                siteMap += "<Niveau_1 Title = \"" + Niveau1Titles[i] + "\" >";
                foreach (var n22 in n2)
                {
                    siteMap += "<Niveau_2 Title = \"" + n22.Title + "\" Link = \"" + n22.Link + "\" >";
                    List<Couple> Niveau3Titles = new List<Couple>();
                    Niveau3Titles = getNiveau3Titles(n22.Link);
                    foreach (var n3 in Niveau3Titles)
                    {
                        siteMap += "<Niveau_3 Title = \"" + n3.Title + "\" Link = \"" + n3.Link + "\" />";
                    }
                    siteMap += "</Niveau_2>";

                }
                siteMap += "</Niveau_1>";
                i++;
            }
            siteMap += "</Root>";


            Console.WriteLine(siteMap);
            Console.ReadLine();
        }
        static List<string> getNiveau1Titles(string aux1)
        {
            return new List<string> { "aaa", "aab", "aac", "aad" };
        }
        static List<List<Couple>> getNiveau2Titles(string aux1)
        {
            return new List<List<Couple>>() { 
                  new List<Couple>() {new Couple() { Title = "T11", Link =  "L11"}, new Couple() { Title = "T12", Link =  "L12"}, new Couple() { Title = "T13", Link =  "L13"} },
                  new List<Couple>() {new Couple() { Title = "T21", Link =  "L21"}, new Couple() { Title = "T22", Link =  "L22"}, new Couple() { Title = "T23", Link =  "L23"} },
                  new List<Couple>() {new Couple() { Title = "T31", Link =  "L31"}, new Couple() { Title = "T32", Link =  "L32"}, new Couple() { Title = "T33", Link =  "L33"} },
                  new List<Couple>() {new Couple() { Title = "T41", Link =  "L41"}, new Couple() { Title = "T42", Link =  "L42"}, new Couple() { Title = "T43", Link =  "L43"} }
            };
        }
        static List<Couple> getNiveau3Titles(string aux1)
        {
            return new List<Couple>() { new Couple() { Title = "T100", Link = "L100" }, new Couple() { Title = "T101", Link = "L101" }, new Couple() { Title = "T102", Link = "L102" } };
        }

    }
    public class Couple
    {
        public string Title { get; set; }
        public string Link { get; set; }
    }

}

Here is the xml results

<?xml version = "1.0" encoding = "UTF-8" ?>
<Root>
  <Niveau_1 Title = "aaa" >
    <Niveau_2 Title = "T11" Link = "L11" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T12" Link = "L12" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T13" Link = "L13" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
  </Niveau_1>
  <Niveau_1 Title = "aab" >
    <Niveau_2 Title = "T21" Link = "L21" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T22" Link = "L22" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T23" Link = "L23" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
  </Niveau_1>
  <Niveau_1 Title = "aac" >
    <Niveau_2 Title = "T31" Link = "L31" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T32" Link = "L32" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T33" Link = "L33" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
  </Niveau_1>
  <Niveau_1 Title = "aad" >
    <Niveau_2 Title = "T41" Link = "L41" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T42" Link = "L42" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
    <Niveau_2 Title = "T43" Link = "L43" >
      <Niveau_3 Title = "T100" Link = "L100" />
      <Niveau_3 Title = "T101" Link = "L101" />
      <Niveau_3 Title = "T102" Link = "L102" />
    </Niveau_2>
  </Niveau_1>
</Root>

They say a picture is worth 1000 words. – jdweng Jul 14 '15 at 03:53 — jdweng, Jul 14 '15 at 03:53

score 0 · Answer 3 · edited May 23 '17 at 10:24

When using lambda expressions or anonymous methods in C#, we have to be wary of the access to modified closure pitfall. For example:

foreach (var s in strings)
{
   query = query.Where(i => i.Prop == s); // access to modified closure
   ...
}

Due to the modified closure, the above code will cause all of the Where clauses on the query to be based on the final value of s.

As explained here, this happens because the s variable declared in foreach loop above is translated like this in the compiler:

string s;
while (enumerator.MoveNext())
{
   s = enumerator.Current;
   ...
}

instead of like this:

while (enumerator.MoveNext())
{
   string s;
   s = enumerator.Current;
   ...
}

As pointed out here, there are no performance advantages to declaring a variable outside the loop, and under normal circumstances the only reason I can think of for doing this is if you plan to use the variable outside the scope of the loop:

string s;
while (enumerator.MoveNext())
{
   s = enumerator.Current;
   ...
}
var finalString = s;

However variables defined in a foreach loop cannot be used outside the loop:

foreach(string s in strings)
{
}
var finalString = s; // won't work: you're outside the scope.

So the compiler declares the variable in a way that makes it highly prone to an error that is often difficult to find and debug, while producing no perceivable benefits.

Did you solve the issue? Not sure what the answer above is trying to tell us. Is it just a response to Enigmativity? — jdweng, Jul 14 '15 at 12:44

C# foreach loop outer string not updating as normal

3 Answers3