Is this an inefficient way to parse XML?

Question

I'm probably worried about improper optimization, but I have such a grumbling thought that it parses the xml tree over and over again, maybe I'm reading it somewhere. I can’t remember.

Anyway, here is what I do:

using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Xml.Linq; using System.Net; namespace LinqTestingGrounds { class Program { static void Main(string[] args) { WebClient webClient = new WebClient(); webClient.DownloadStringCompleted += new DownloadStringCompletedEventHandler(webClient_DownloadStringCompleted); webClient.DownloadStringAsync(new Uri("http://www.dreamincode.net/forums/xml.php?showuser=335389")); Console.ReadLine(); } static void webClient_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e) { if (e.Error != null) { return; } XDocument xml = XDocument.Parse(e.Result); User user = new User(); user.ID = xml.Element("ipb").Element("profile").Element("id").Value; user.Name = xml.Element("ipb").Element("profile").Element("name").Value; user.Rating = xml.Element("ipb").Element("profile").Element("rating").Value; user.Photo = xml.Element("ipb").Element("profile").Element("photo").Value; user.Reputation = xml.Element("ipb").Element("profile").Element("reputation").Value; user.Group = xml.Element("ipb").Element("profile").Element("group").Element("span").Value; user.Posts = xml.Element("ipb").Element("profile").Element("posts").Value; user.PostsPerDay = xml.Element("ipb").Element("profile").Element("postsperday").Value; user.JoinDate = xml.Element("ipb").Element("profile").Element("joined").Value; user.ProfileViews = xml.Element("ipb").Element("profile").Element("views").Value; user.LastActive = xml.Element("ipb").Element("profile").Element("lastactive").Value; user.Location = xml.Element("ipb").Element("profile").Element("location").Value; user.Title = xml.Element("ipb").Element("profile").Element("title").Value; user.Age = xml.Element("ipb").Element("profile").Element("age").Value; user.Birthday= xml.Element("ipb").Element("profile").Element("birthday").Value; user.Gender = xml.Element("ipb").Element("profile").Element("gender").Element("gender").Element("value").Value; Console.WriteLine(user.ID); Console.WriteLine(user.Name); Console.WriteLine(user.Rating); Console.WriteLine(user.Photo); Console.WriteLine(user.Reputation); Console.WriteLine(user.Group); Console.WriteLine(user.Posts); Console.WriteLine(user.PostsPerDay); Console.WriteLine(user.JoinDate); Console.WriteLine(user.ProfileViews); Console.WriteLine(user.LastActive); Console.WriteLine(user.Location); Console.WriteLine(user.Title); Console.WriteLine(user.Age); Console.WriteLine(user.Birthday); Console.WriteLine(user.Gender); //Console.WriteLine(xml); } } } 

Is it Good Enough ™ or is there a much faster way to make out what I need?

ps. I do most of the operations in the DownloadStringCompleted event, should I not do this? First time using this method. Thanks!

Answer 1

I don’t know about efficiency, but for readability use the profile variable instead of iterating over all things over and over:

  User user = new User(); var profile = xml.Element("ipb").Element("profile"); user.ID = profile.Element("id").Value; 

Answer 2

I believe that XML serialization is a way to solve this problem. As long as your properties match the xml elements, this will be trivial. Otherwise, you just need to map them using the XmlElement and XmlAttribute attribute classes. Here is a simple code for ordinary xml deserialization into a class:

 public T Deserialise(string someXml) { XmlSerializer reader = new XmlSerializer(typeof (T)); StringReader stringReader = new StringReader(someXml); XmlTextReader xmlReader = new XmlTextReader(stringReader); return (T) reader.Deserialize(xmlReader); } 

Answer 3

I will add the answer to Oded: another way to increase readability is to use the XPathSelectElement extension method .

So your code will look like this:

 user.ID = xml.XPathSelectElement("ipb/profile/id").Value; 

Answer 4

No, it does not parse XML over and over; once when you call

 XDocument.Parse(e.Result); 

After that, the calls refer to the tree structure of the xml object.

“Analysis” means analyzing an unstructured text string (for example, from a file) and creating data structures (such as a tree). Your calls ... .Element("foo") not processed, but refer to parts of the data structure that were created by calling XDocument.Parse() .

If you are wondering if your code will repeat some steps redundantly and can be optimized, then yes, you are moving ipb/profile excessively. This is not parsing, but Element ("foo") calls should do some work comparing string arguments with child element names. @ However, the proposal corrects this for reasons of readability, but also helps in efficiency.