Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

I just finished my working code, but still want to improve it.

I want to transform this input:

<item>asdf</item>
<item>asdf</item>
<item>asdf</item>

to this output:

<string name="x1">asdf</string>
<string name="x2">asdf</string>
<string name="x3">asdf</string>

My current code:

   static void Main(string[] args)
        {
            String content;

            using (StreamReader reader = new StreamReader("arrays2.xml"))
            {
                content = reader.ReadToEnd();
            }

            int counter = 0;
            int startIndex = 0;
            while ((startIndex = content.IndexOf("<item>", startIndex)) != -1)
            {
                counter++;           
                content = content.Substring(0, startIndex) + "<string name=\"rule" + counter + "\">" + content.Substring(startIndex + 6);
            }

            content = content.Replace("</item>", "</string>");


            using (StreamWriter writer = new StreamWriter("arrays2_formatted.xml"))
            {
                writer.Write(content);
            }
            Console.WriteLine(content);


        }

I think the bottleneck is in the content assignment within the loop, as a lot of String instance are created and thrown away.

However, is there a completely different way to do this efficiently?

share|improve this question
So, you're saying your code is too slow? Have you profiled it? How big is the file you're converting? Are you sure the code is the bottleneck and not the disk? – svick May 6 '12 at 9:00
@svick No, I don't claim anything. I just want to improve it, as I am not a c# developer, normally. :) – Michael May 6 '12 at 12:14

1 Answer

up vote 2 down vote accepted

Don't parse it through string manipulation, use the XML libraries in the framework. I'd recommend LINQ to XML. Parse it and make your changes.

var xmlStr = @"<root>
<item>asdf</item>
<item>asdf</item>
<item>asdf</item>
</root>";

var doc = XDocument.Parse(xmlStr);         // parse from string
//var doc = XDocument.Load("arrays2.xml"); // or load from file
var items = doc.Descendants("item");       // find all item elements
var index = 1;
foreach (var item in items.ToList())  // ToList() required since we're modifying the document
{
    var name = "x" + index++;
    var nameAttr = new XAttribute("name", name);           // create the name attribute
    var value = (string)item;                              // read the value as a string
    var newNode = new XElement("string", nameAttr, value); // create the new element
    item.ReplaceWith(newNode);                             // replace the current one
}
//doc.Save("arrays2_formatted.xml"); // save the changes to the file
share|improve this answer
Note, your "XML" is not valid XML, it needs a root. – Jeff Mercado May 6 '12 at 8:27

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.