Foreach not working properly

Question

I'm trying to write a method that takes some elements from Unity .config file. Each element has type and mapTo attributes, and I'm taking their string values, getting substrings I need and putting them in two separated lists. After that I want to write the content of lists in some data grid.

The problem is that after a finishing all foreach loops code goes back into the one for checking single register and enters again in other two for types and mapTo values. In other words in lists I get in-countless string values instead of only getting them once. I'm a beginner and I've tried lot of things, but nothing seams to do a job. Anybody has an idea what I'm getting wrong? The code of method in C# looks like this:

private void ReadAdvancedConfigFile(string path)
        {
            XElement root = null;
            root = XElement.Load(new XmlTextReader(path));

            if (root != null)
            {
                XNamespace ns = "http://schemas.microsoft.com/practices/2010/unity";
                var registers = root.Element(ns + "unity").Element(ns + "container").Descendants(ns + "register");

                if (registers.Count() > 0)
                {
                    var tipList = registers.Select(x => x.Attribute("type").Value);
                    var mapToList = registers.Select(x => x.Attribute("mapTo").Value);
                    List<string> listresult = new List<string>();
                    List<string> listresultm = new List<string>();

                    foreach (var reg in registers)
                    {
                        foreach (var tpl in tipList)
                        {
                            var end = tpl.IndexOf(',');
                            var start = tpl.LastIndexOf('.', (end == -1 ? tpl.Length - 1 : end)) + 1;
                            var result = tpl.Substring(start, (end == -1 ? tpl.Length : end) - start);
                            listresult.Add(result);
                        }
                        foreach (var mpl in mapToList)
                        {
                            var endm = mpl.IndexOf(',');
                            var startm = mpl.LastIndexOf('.', (endm == -1 ? mpl.Length - 1 : endm)) + 1;
                            var resultm = mpl.Substring(startm, (endm == -1 ? mpl.Length : endm) - startm);
                            listresultm.Add(resultm);
                        }

                        int maxLenList = Math.Max(listresult.Count, listresultm.Count);
                        for (int i = 0; i < maxLenList; i++)
                        {
                            if (i < listresult.Count && i < listresultm.Count)
                            {
                                _obsCollection.Add(new Tuple<string, string>(listresult[i], listresultm[i]));
                            }
                            else if (i >= listresult.Count)
                            {
                                _obsCollection.Add(new Tuple<string, string>(string.Empty, listresultm[i]));
                            }
                            else if (i >= listresultm.Count)
                            {
                                _obsCollection.Add(new Tuple<string, string>(listresultm[i], string.Empty));
                            }
                        }
                    }
                    tabela.ItemsSource = _obsCollection;
                }
            }
        }

Method gets call from a button named Load that finds Unity.config file from some location in file system like this:

 private void button1_Click(object sender, RoutedEventArgs e)
        {
            OpenFileDialog fDialog = new OpenFileDialog();

            fDialog.Title = "Open XML file";
            fDialog.Filter = "XML files|*.config";
            fDialog.InitialDirectory = @"C:\";

            bool? control = fDialog.ShowDialog();
            if (control.Value)
            {
                var filePath = fDialog.FileName;
                ReadAdvancedConfigFile(filePath);
            }

        }

And in Unity.config is XML file with this format(I'm deleted most of elements, to same some space here, it will work with this too):

<?xml version="1.0" encoding="utf-8" ?>
<configuration>

    <configSections>
        <section name="unity" type="Microsoft.Practices.Unity.Configuration.UnityConfigurationSection, Microsoft.Practices.Unity.Configuration"/>
    </configSections>

    <unity xmlns="http://schemas.microsoft.com/practices/2010/unity">

        <container name="container">

            <register name="configService" type="Web.Common.Interfaces.IConfigService, Web.Common"
                      mapTo="Web.Common.Services.ConfigServiceImpl, Web.Common">
                <lifetime type="singleton" />
                <constructor>
                    <param name="res" value="Resources.ClientStrings"> </param>
                    <param name="configFile" value="webclient.config"> </param>
                </constructor>
                <!--<property name="LocalisationService" dependencyName="LocalisationService" />-->
                <!--This is a property injection from the language plugin -->
            </register>

            <register name="scaleCoefConfigService" type="Web.WebClient.Services.IScaleCoefConfigService, Web.WebClient.TDMSWebApp"
                      mapTo="Web.WebClient.Services.Implementations.ScaleCoefConfigServiceImpl, Web.WebClient.TDMSWebApp">
                <lifetime type="singleton" />
                <constructor>
                    <param name="configService">
                        <dependency name="configService"/>
                    </param>
                </constructor>
            </register>

            <register name="sessionService" type="Web.Common.Interfaces.ISessionService, Web.Common" 
                      mapTo="Web.Common.Services.SessionServiceImpl, Web.Common">
                <lifetime type="singleton" />
            </register>

            <register name="licenseManagerService" type="Web.Common.Interfaces.ILicenseManagementService, Web.Common"
                      mapTo="Web.Common.Services.LicenseManagementServiceImpl, Web.Common">
                <lifetime type="singleton" />
            </register>
        </container>
    </unity>
</configuration>

We can't find bugs in your code when we can't even run it. Please, learn to debug your code : http://www.youtube.com/watch?v=C0vDKXIq_9A — Euphoric, Feb 05 '14 at 08:20
As Euphoric says: narrow down your code. Create a demo project, remove all code that does not contribute to the problem and post it here. — Patrick Hofman, Feb 05 '14 at 08:21
I'm doing a debug and still can't realize whats wrong. Thanks anyway...I will continue to try :) — nemo_87, Feb 05 '14 at 08:22
@PatrickHofman Ok, I will try to do that in next post. Hope I'm gonna be able to give you a nicer look into code. :) — nemo_87, Feb 05 '14 at 08:23
Try it this time. How would you expect us to help you when we having nothing to go with? — Patrick Hofman, Feb 05 '14 at 08:28

score 2 · Answer 1 · answered Feb 05 '14 at 08:21

2

Looks to me as if you don't need the foreach loop. You're already fetching all 'type' and 'mapTo' attributes when you query using LINQ:

var tipList = registers.Select(x => x.Attribute("type").Value);
var mapToList = registers.Select(x => x.Attribute("mapTo").Value);

This effectively gives you all the attributes of the xelements in 'registers'. You're not even using the var 'reg' within your loop...

answered Feb 05 '14 at 08:21

Reinder Wit

6,490
1
25
36

Thanks for responding. I will try that right away and let you know did it worked :) – nemo_87 Feb 05 '14 at 08:32
It does work when you remove `foreach (var reg in registers)` – Patrick Hofman Feb 05 '14 at 08:42
@Reinder Tried to remove foreach(var reg in registers), but problem remains. I've totally messed up something real bad. :-/ But you were right, that loop didn't do anything, it was totally unnecessary. After exiting both two foreach loops it goes back to var tipList and var mapToList and copy values in lists again and again. – nemo_87 Feb 05 '14 at 08:43

score 1 · Accepted Answer · answered Feb 05 '14 at 08:45

I adjusted your code and now it works (removed foreach):

XElement root = null;
root = XElement.Load(new XmlTextReader(path));

if (root != null)
{
    XNamespace ns = "http://schemas.microsoft.com/practices/2010/unity";
    var registers = root.Element(ns + "unity").Element(ns + "container").Descendants(ns + "register");

    if (registers.Count() > 0)
    {
        var tipList = registers.Select(x => x.Attribute("type").Value);
        var mapToList = registers.Select(x => x.Attribute("mapTo").Value);
        List<string> listresult = new List<string>();
        List<string> listresultm = new List<string>();

        foreach (string tpl in tipList)
        {
            int end = tpl.IndexOf(',');
            int start = tpl.LastIndexOf('.', (end == -1 ? tpl.Length - 1 : end)) + 1;
            string result = tpl.Substring(start, (end == -1 ? tpl.Length : end) - start);
            listresult.Add(result);
        }

        foreach (string mpl in mapToList)
        {
            int endm = mpl.IndexOf(',');
            int startm = mpl.LastIndexOf('.', (endm == -1 ? mpl.Length - 1 : endm)) + 1;
            string resultm = mpl.Substring(startm, (endm == -1 ? mpl.Length : endm) - startm);
            listresultm.Add(resultm);
        }

        int maxLenList = Math.Max(listresult.Count, listresultm.Count);

        for (int i = 0; i < maxLenList; i++)
        {
            if (i < listresult.Count && i < listresultm.Count)
            {
                _obsCollection.Add(new Tuple<string, string>(listresult[i], listresultm[i]));
            }
            else if (i >= listresult.Count)
            {
                _obsCollection.Add(new Tuple<string, string>(string.Empty, listresultm[i]));
            }
            else if (i >= listresultm.Count)
            {
                _obsCollection.Add(new Tuple<string, string>(listresultm[i], string.Empty));
            }
        }
    }
}

yeah that totally worked, thanks :) So I have one question. You changed almost all var to string and int types. Is there some catch with that? Like is there problem if we are using too much var for defining or something like that? P.S. I have a new problem now, instead of getting those strings from list in datagrid I get some lines. But that's totally new problem :) — nemo_87, Feb 05 '14 at 08:55
Sorry, I changed the `var` variables just for myself to make clear what was happening. I personally dislike `var`. — Patrick Hofman, Feb 05 '14 at 08:56
@nemo_87: Post an example in a new question and I will take a look. — Patrick Hofman, Feb 05 '14 at 08:57
Oh ok, I was thinking that maybe is not good using too much var. :) I will new post it right now, I just need to study a code little bit better, so I can ask a right question. :) — nemo_87, Feb 05 '14 at 08:59
I've posted new post and named it: Passing string values from list to datagrid Gonna try to solve this problem, but if you get some idea thanks a lot :) — nemo_87, Feb 05 '14 at 09:15

Foreach not working properly

2 Answers2