current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

This seems like rather too much nesting:

using (XmlReader reader = XmlReader.Create(filename))
{
    while (reader.Read())
    {
        if (reader.IsStartElement())
        {
            switch (reader.Name)
            {
                case "Width":
                    map.Width = ParseXMLValue(reader);
                    break;
                case "Height":
                    map.Height = ParseXMLValue(reader);
                    break;
                case "TileSize":
                    map.TileSize = ParseXMLValue(reader);
                    break;
                case "Layers":
                    map.LayerCount = ParseXMLValue(reader);
                    break;
                case "Layout":
                    ParseLayout(reader);
                    break;
                case "Layer":
                    currentLayerIndex = ParseLayer(reader);
                    break;
                case "CollisionLayer":
                    currentLayerIndex = ParseCollisionLayer();
                    break;
                case "Row":
                    ParseRow(reader);
                    break;
            }
        }
    }
}

This is ParseXMLValue(reader):

private int ParseXMLValue(XmlReader reader)
{
    reader.Read();

    return int.Parse(reader.Value);
}

I'm new to reading XML in C#. Surely there is a better way?

share|improve this question

2 Answers 2

up vote 5 down vote accepted

Yes, much. XDocument is the easier way. Though it does not perform as well if your documents are of significant size or performance is absolutely imperative: http://www.nearinfinity.com/blogs/joe_ferner/performance_linq_to_sql_vs.html

It works like:

XDocument someXmlDoc = XDocument.Create(fileName);
IEnumerable<XElement> widthElements = someXmlDoc.Descendants("Width");
int[] widthValues = widthElements.Select(xelement => int.Parse(xelement.Value)).ToArray();

Though I don't understand the logic in your snippet as you're setting elements of the same member repeatedly, so I'm not going to reserve giving a more eleborate use case that might match your functionality better, though I would suggest reading the msdn articles for IEnumerable.Select() and IEnumerable.Where() to get an idea how to use it for your particular purposes.

Select: http://msdn.microsoft.com/en-us/library/bb548891.aspx

Where: http://msdn.microsoft.com/en-us/library/bb534803.aspx

share|improve this answer
    
Is this actually more performant than the forward looking XMLReader approach? I haven't yet had to use Linq2XML, but it's hard to imagine that random access could be faster than a stream-like reader. Interesting, I'll have to run some tests as we use an XML based API at work which involves a fair amount of parsing. –  Ed S. Aug 23 '11 at 23:01
    
@Ed sorry my memory fails me, I was thinking of the old XmlDocument not XmlReader, XmlReader is quite performant: nearinfinity.com/blogs/joe_ferner/… –  Jimmy Hoffa Aug 23 '11 at 23:09
    
Ok, that makes sense. The old DOM based XML classes are relatively slow. –  Ed S. Aug 24 '11 at 2:05
up vote 0 down vote

I always prefer working with POCO objects instead of Xml documents. If I correctly understand, you just want to parse an xml file into a set of objects.

To do so you need to:

  1. Define objects model - that's it, just create a set of classes you want to get after deserialization. There are should be a root class that stands for the root xml element.

  2. Put correct attributes into the properties of the classes. That's how you will bind properties with xml elements/attributes

  3. Use XmlSerializer to deserialize the file into a normal POCO classes.

See a good explanation here - http://www.agiledeveloper.com/articles/XMLSerialization.pdf

share|improve this answer
    
I agree, but you can't always control the format of the XML coming in. Built in serialization is great for saving state between sessions or generating things only your app will consume, but (for example) if your XML is coming from some WebService there is very little chance that it will "just work". –  Ed S. Aug 24 '11 at 21:59
    
I agree, that it might be tricky sometimes. But in the core, your web service consumer should use xml only as intermediate data source. It might be not as easy as I explained to get domain model objects from xml, but we should aim for it... –  Sergei Aug 25 '11 at 6:11

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.