LINQ Refactoring

Question

I refactored my foreach loop from this before:

foreach (KeyValuePair[string, string] param in paramsList)
{
    XmlElement mainNode = xmlDoc.CreateElement("parameter");
    mainNode.SetAttribute("name", param.Key);
    mainNode.SetAttribute("value", param.Value);
    rootNode.AppendChild(mainNode);
}

to this, using LINQ:

XmlElement mainNode = xmlDoc.CreateElement("parameter");
var selected = paramsList.AsEnumerable().Select(param => param).ToList();
selected.ForEach(x => (mainNode.SetAttribute("name", x.Key)));
selected.ForEach(x => (mainNode.SetAttribute("value", x.Value)));
rootNode.AppendChild(mainNode);

However, i know the section below can still be refactored into a single loop but i dont know how. please enlighten me.

selected.ForEach(x => (mainNode.SetAttribute("name", x.Key)));
selected.ForEach(x => (mainNode.SetAttribute("value", x.Value)));

Also, your original code would yield as many "parameter" elements as there are elements in the paramsList, while refactored code will always produce exactly one "parameter" element, with "name" and "value" attributes being set length(paramsList) times on it. I don't think you have it refactored correctly. — THX-1138, May 04 '09 at 12:39
yeah you're absolutely right. anyways, i just commented on bruno conde's code and now id rather not replace my existing foreach loop. — Cante Ibanez, May 04 '09 at 13:33

score 3 · Accepted Answer · answered May 04 '09 at 12:23

I think you can achieve the same results with:

        paramsList.ToList().ForEach( e => {
            XmlElement mainNode = xmlDoc.CreateElement("parameter");
            mainNode.SetAttribute("name", e.Key);
            mainNode.SetAttribute("value", e.Value);
            rootNode.AppendChild(mainNode);
        });

but, in this case, I would choose a simple foreach:

        foreach (var e in paramsList)
        {
            XmlElement mainNode = xmlDoc.CreateElement("parameter");
            mainNode.SetAttribute("name", e.Key);
            mainNode.SetAttribute("value", e.Value);
            rootNode.AppendChild(mainNode);
        }

i see.. so you could already do that. Seeing your code made me choose the simple foreach loop now. It's because compare to the simple foreach loop I won't burden my code anymore to create a Generic.List and also won't delegate anymore. Thanks! — Cante Ibanez, May 04 '09 at 13:31

score 2 · Answer 2 · answered May 04 '09 at 12:21

2

maybe something like this

selected.ForEach(x => 
          { 
             mainNode.SetAttribute("name", x.Key);
             mainNode.SetAttribute("value", x.Value);
          });

answered May 04 '09 at 12:21

John Boker

82,559
17
97
130

Joel Mueller · Answer 3 · 2009-05-05T02:19:51.163

Any chance you could switch from XmlDocument to XDocument? LINQ to XML integrates much better with LINQ, as you might expect.

var nodes = from pair in paramsList
            select new XElement("parameter",
                                new XAttribute("name", pair.Key),
                                new XAttribute("value", pair.Value));

And that's it, except for adding the nodes to the document, or passing them into an XDocument constructor or something.

Edit: To clarify, your question is tagged "linqtoxml", but LINQ to XML implies a specific set of classes in the System.Xml.Linq namespace, such as XDocument, XElement, and XAttribute. Your sample code isn't using any actual LINQ to XML classes, and I'm suggesting that if you want to use LINQ to build your XML, the actual LINQ to XML classes would serve you better than XmlDocument and friends.

LINQ Refactoring

3 Answers3