How can I improve the performance and design of this code

Question

Down below is my code I'm currently using, but is there a way to make it cleaner and shorter? I want my code to look short efficient and nice looking. My teacher said something with "do it with a loop" but I can't figure out what that loop does and how it would work.

        {
            XmlWriter xmlWriter = XmlWriter.Create(PATH);

            xmlWriter.WriteStartDocument();
            xmlWriter.WriteStartElement("Config");

            xmlWriter.WriteStartElement("TBSOMS");
            xmlWriter.WriteString(TBSOMS.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBWVB");
            xmlWriter.WriteString(TBWVB.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBWNB");
            xmlWriter.WriteString(TBWNB.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBASPMM1");
            xmlWriter.WriteString(TBASPMM1.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBASPMM2");
            xmlWriter.WriteString(TBASPMM2.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBDUM");
            xmlWriter.WriteString(TBDUM.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBADPR");
            xmlWriter.WriteString(TBADPR.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteStartElement("TBAR");
            xmlWriter.WriteString(TBAR.Text);
            xmlWriter.WriteEndElement();

            xmlWriter.WriteEndDocument();

            xmlWriter.Close();
        }

as a starting point you could store the values in a List of keypairs or strings and loop through that with a foreach loop. see https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.keyvaluepair-2?view=netframework-4.8 — level_zebra, Sep 18 '19 at 09:51

score 1 · Answer 1 · answered Sep 18 '19 at 09:57

I'd start with something like this...

var values = new Dictionary<string, string>
{
    { nameof(TBWVB), TBWVB.Text },
    { nameof(TBWNB), TBWNB.Text }
    //        .... etc ......
}

XmlWriter xmlWriter = XmlWriter.Create(PATH);

xmlWriter.WriteStartDocument();
xmlWriter.WriteStartElement("Config");

foreach (var item in values.Keys)
{
    xmlWriter.WriteStartElement(item);
    xmlWriter.WriteString(values[item]);
    xmlWriter.WriteEndElement();
}

ape · Answer 2 · 2019-09-18T10:07:42.647

1

Something in the like of :

private void writeElement(XmlWriter xmlwrtr, String element, var obj){
        xmlwrtr.WriteStartElement(element);
        xmlwrtr.WriteString(obj.Text);
        xmlwrtr.WriteEndElement();
}

Which would give :

 {
        XmlWriter xmlWriter = XmlWriter.Create(PATH);
        xmlWriter.WriteStartDocument();
        xmlWriter.WriteStartElement("Config");

        writeElement(xmlWriter, "TBSOMS", TBSOMS)
        writeElement(xmlWriter, "TBWVB", TBWVB)
        // And so on

        xmlWriter.WriteEndDocument();

        xmlWriter.Close()
}

edited Sep 18 '19 at 10:07

answered Sep 18 '19 at 09:57

ape

21
5

there is no need to pass the elements name as a parameter if it is the objects name ([see my answer](https://stackoverflow.com/a/57990003/10173803)). Wait nevermind, i just had an insane brain lag – JieBaef Sep 18 '19 at 10:00
i've updated my answer, but then you have updated your comment, is it still valid ? Also, feel free to edit my answer :) – ape Sep 18 '19 at 10:03
not exactly, it will just pass "obj" as the variables name so you need to pass the actual objects name as a parameter, i updated my answer and now it should work fine – JieBaef Sep 18 '19 at 10:04

JieBaef · Answer 3 · 2019-09-18T10:02:59.617

By creating a class variable you can access it's values in multiple methods.

private static XmlWriter xmlWriter;
public void Write()
{
    xmlWriter = XmlWriter.Create(Path);
    xmlWriter.WriteStartDocument();
    xmlWriter.WriteStartElement("Config");

    new List<(elementDataType element, string name)> // note that you need to change "elementDataType" to your elements data type
    {
        (TBSOMS, nameof(TBSOMS)),
        (TBWVB, nameof(TBWVB)),
        // ... just list all elements in here
    }.ForEach(elem => WriteElement(elem.element, elem.name));


    xmlWriter.WriteEndDocument();
    xmlWriter.Close();
}

private void WriteElement(var element, string name) //note that you need to change var to your elements datatype
{
    xmlWriter.WriteStartElement(name);
    xmlWriter.WriteString(element.Text);
    xmlWriter.WriteEndElement();
}

How can I improve the performance and design of this code

3 Answers3