1

Can someone tell me what I have to change to fix this foreach loop (code below). It only runs once and the counter gives me the correct amount of attributes.

public static String FindChanges(string input, string old)
    {
        XDocument newXml = XDocument.Parse(input);
        XDocument returnXML = newXml;
        XDocument oldXml = XDocument.Parse(old);
        int counter = newXml.Root.Attributes().Count();
        foreach (XAttribute check in newXml.Root.Attributes())
        {
            if (check.Value == oldXml.Root.Attribute(check.Name).Value) returnXML.Root.Attribute(check.Name).Remove();
        }
        return returnXML.ToString();
    }

Original the code was:

public static String FindChanges(string input, string old)
    {
        XDocument newXml = XDocument.Parse(input);
        XDocument oldXml = XDocument.Parse(old);
        int counter = newXml.Root.Attributes().Count();
        foreach (XAttribute check in newXml.Root.Attributes())
        {
            if (check.Value == oldXml.Root.Attribute(check.Name).Value) newXML.Root.Attribute(check.Name).Remove();
        }
        return newXML.ToString();
    }

but trying to fix it I've changed it to the code above. The foreach only removes the first attribute, but It has to remove all the attributes which are the same.

as requested the XML files, first the new XML:

<User LoginName=\"Paul\" Owner=\"\" UserId=\"Paul\" Alias=\"Testing\" UserType=\"PAID\" ClientType=\"OBM\" Status=\"ENABLE\" Quota=\"104857600\" Timezone=\"GMT-08:00 (PST)\" Language=\"en\" DataFile=\"0\" DataSize=\"0\" RetainFile=\"0\" RetainSize=\"0\" UncompressedSize=\"0\" UncompressedRetainSize=\"0\" EnableMSSQL=\"Y\" EnableMSExchange=\"N\" MsExchangeQuota=\"0\" EnableOracle=\"N\" EnableLotusNotes=\"N\" EnableLotusDomino=\"Y\" EnableMySQL=\"Y\" EnableInFileDelta=\"N\" EnableShadowCopy=\"N\" EnableExchangeMailbox=\"Y\" ExchangeMailboxQuota=\"1\" EnableNASClient=\"Y\" EnableDeltaMerge=\"Y\" EnableMsVm=\"Y\" MsVmQuota=\"2\" EnableVMware=\"N\" VMwareQuota=\"0\" Bandwidth=\"0\" Notes=\"BLABLjbldiela!!\" UserHome=\"/ubs/module/obsr/system/obsr/user/Paul\" RegistrationDate=\"1417186638067\" MailboxUsage=\"0\" SuspendPaidUser=\"N\" SuspendPaidUserDate=\"20150212\" LastBackupDate=\"0\" EnableCDP=\"N\" EnableShadowProtectBareMetal=\"N\" EnableWinServer2008BareMetal=\"Y\" MUserId=\"5\" CompanyId=\"2\" CompanyName=\"NoCompany\">\r\n  <Contact Name=\"demo\" Email=\"demo2@home.nl\" />\r\n</User>

and second the old XML:

<User LoginName=\"Paul\" Owner=\"\" UserId=\"Paul\" Alias=\"Testing\" UserType=\"PAID\" ClientType=\"OBM\" Status=\"ENABLE\" Quota=\"104857600\" Timezone=\"GMT-08:00 (PST)\" Language=\"en\" DataFile=\"0\" DataSize=\"0\" RetainFile=\"0\" RetainSize=\"0\" UncompressedSize=\"0\" UncompressedRetainSize=\"0\" EnableMSSQL=\"Y\" EnableMSExchange=\"N\" MsExchangeQuota=\"0\" EnableOracle=\"N\" EnableLotusNotes=\"N\" EnableLotusDomino=\"Y\" EnableMySQL=\"Y\" EnableInFileDelta=\"N\" EnableShadowCopy=\"N\" EnableExchangeMailbox=\"Y\" ExchangeMailboxQuota=\"1\" EnableNASClient=\"Y\" EnableDeltaMerge=\"Y\" EnableMsVm=\"Y\" MsVmQuota=\"2\" EnableVMware=\"N\" VMwareQuota=\"0\" Bandwidth=\"0\" Notes=\"BLABLjbldiela!!\" UserHome=\"/ubs/module/obsr/system/obsr/user/Paul\" RegistrationDate=\"1417186638067\" MailboxUsage=\"0\" SuspendPaidUser=\"N\" SuspendPaidUserDate=\"20150212\" LastBackupDate=\"0\" EnableCDP=\"N\" EnableShadowProtectBareMetal=\"N\" EnableWinServer2008BareMetal=\"Y\" MUserId=\"5\" CompanyId=\"1\" CompanyName=\"NoCompany\">\r\n  <Contact Name=\"demo\" Email=\"demo2@home.nl\" />\r\n</User>

This is a work in progress and the function for now will not work correctly and won't filter the emails, but it should only return the changed variable CompanyId.

J. Steen
  • 15,470
  • 15
  • 56
  • 63
Quispie
  • 948
  • 16
  • 30
  • 3
    Use a forloop and update your index when you remove an item, your original code is modifiying the list you iterate over – Sayse Mar 12 '15 at 11:40
  • 2
    Related: [How to remove elements from a generic list while iterating over it?](http://stackoverflow.com/questions/1582285/how-to-remove-elements-from-a-generic-list-while-iterating-over-it) – Sayse Mar 12 '15 at 11:41
  • Thanks for all the responses and as requested I've added the xml files. I also added the working code and upload the final version later. – Quispie Mar 12 '15 at 15:13
  • Please don't put solutions in the question. Post **all** relevant information to your solution as an answer. – J. Steen Mar 12 '15 at 15:22
  • Well I'm sorry I guess... I'm just trying to be helpfull and make a nice overview. – Quispie Mar 12 '15 at 15:34

2 Answers2

1

As promised the final code:

/// <summary>
    /// Find the changes in 
    /// </summary>
    /// <param name="input">The new Input XML which is saved in the databases</param>
    /// <param name="old">The old XML which was loaded at the start</param>
    /// <returns>only the diffrences in XML format</returns>
    public static String FindChanges(string input, string old)
    {
        bool returnEmpty = true;
        XDocument newXml = XDocument.Parse(input);
        XDocument returnXML = XDocument.Parse(input);
        XDocument oldXml = XDocument.Parse(old);
        foreach (XAttribute check in newXml.Root.Attributes())
        {
            if (check.Value == oldXml.Root.Attribute(check.Name).Value) returnXML.Root.Attribute(check.Name).Remove();
        }
        if (returnXML.Root.HasAttributes) returnEmpty = false;
        if (newXml.Root.HasElements)
        {
            foreach (XElement sub in newXml.Root.Elements())
            {
                foreach (XAttribute check in sub.Attributes())
                {
                    if (check.Value == oldXml.Root.Element(sub.Name).Attribute(check.Name).Value) returnXML.Root.Element(sub.Name).Attribute(check.Name).Remove();
                }
                if (!returnXML.Root.Element(sub.Name).HasAttributes) returnXML.Root.Element(sub.Name).Remove();
                else returnEmpty = false;
            }
        }
        return returnEmpty? "" : returnXML.ToString();
    }
Quispie
  • 948
  • 16
  • 30
0

foreach loop is never a preferable option when we have to modify the items during loop. I have also encountered this issue in the past. And as Sayse has mentioned in his comment, for loop will do (we will have to use an iterator variable to keep the record obviously)

Failed Scientist
  • 1,977
  • 3
  • 29
  • 48
  • I didn't know the exact reason back then (in 2011) as http://stackoverflow.com/questions/1582285/how-to-remove-elements-from-a-generic-list-while-iterating-over-it mentions, but I used pretty much same approach there – Failed Scientist Mar 12 '15 at 11:47