0

I'm currently creating a program that write some XML, but due to the library I'm using I have to create many variables like:

XmlDocument xmlWriter = new XmlDocument();
XmlNode root = xmlWriter.CreateElement("score");
xmlWriter.AppendChild(root);

XmlNode apiKey = xmlWriter.CreateElement("APIkey");
apiKey.InnerText = "placeholder";
root.AppendChild(apiKey);

I have many elements to create just as the API Key, resulting in a lot of variables. So i thought of surrounding each element with braces just for readability like so:

XmlDocument xmlWriter = new XmlDocument();
XmlNode root = xmlWriter.CreateElement("score");
xmlWriter.AppendChild(root);

{
    XmlNode apiKey = xmlWriter.CreateElement("APIkey");
    apiKey.InnerText = "placeholder";
    root.AppendChild(apiKey);
}
apiKey.DoSomething() // Doesn't work because it's out of scope

I believe it's saving some memory and helping me later because the variables don't exist anymore (useful for IntelliSense)

You could say "Just use a function" but I only use this code in this particular part (and calling a function with parameters take some nearly negligeable time but anyway...)

What's your opinion about this ?

StuartLC
  • 104,537
  • 17
  • 209
  • 285
  • 3
    I would make the code simpler by using LINQ to XML instead - do you *have* to use XmlDocument? – Jon Skeet Nov 16 '17 at 10:44
  • 1
    "I believe its saving some memory" - [not so - using braces has no affect on the lifespan of an object within the same function](https://stackoverflow.com/a/27458173/314291), I also made that mistake a while back – StuartLC Nov 16 '17 at 10:49
  • Just use a method. Make it private and if there will be a need for this somewhere else, extract it to a separate class. – FCin Nov 16 '17 at 10:58

1 Answers1

1

I don't see the point of this. Using a single function to create an element and append it to another element (perhaps with params attributes) seems like the way to go here. It will produce a much more readable code. Something like this:

XmlNode CreateAndAppendChild(XmlDocument xmlWriter, XmlNode parent, string name, string innerText, params Tuple<string, string> attributes)
{
    var child = xmlWriter.CreateElement(name);
    foreach(var attribute in attributes)
    {
        child.SetAttribute(attribute.Item1, attribute.Item2);
    }
    child.InnerText = innerText;
    parent.AppendChild(child);
    return child;
}

And the usage:

CreateAndAppendChild(xmlWriter, root, "APIkey", "placeholder");

Instead of your current code:

XmlNode apiKey = xmlWriter.CreateElement("APIkey");
apiKey.InnerText = "placeholder";
root.AppendChild(apiKey);

If you want to add a child element to APIKey:

var apiKey = CreateAndAppendChild(xmlWriter, root, "APIkey", "placeholder");

CreateAndAppendChild(xmlWriter, apiKey , "child", "" Tuple.Create("attributeName", "attributeValue"));
Zohar Peled
  • 79,642
  • 10
  • 69
  • 121
  • Yea, I think I'll do something like that. My functino will ba a bit different though beause I have nested element but it's managable. Thanks :) – Romain Foucher Nov 16 '17 at 11:07
  • You can use this to add a child to any element, not only the root. That's why it takes a reference to the parent... – Zohar Peled Nov 16 '17 at 11:16
  • I've edited my answer with a usage sample (and also changed the function to accept also inner text and return the child node it creates). – Zohar Peled Nov 16 '17 at 11:22