views:

201

answers:

4
var subset = from item in document.Descendants("Id")
             where item.Value == itemId.ToString()
             select new PurchaseItem() {
                 Id = int.Parse(item.Parent.Element("Id").Value),
                 Name = item.Parent.Element("Name").Value,
                 Description = item.Parent.Element("Description").Value,
                 Price = int.Parse(item.Parent.Element("Price").Value)
             };

The structure of the XML is as follows:

<Items>
    <Item>
        <Id></Id>
        <Name></Name>
        <Description></Description>
        <Price></Price>
    </Item>
</Items>

Id, and price are both integer values. Name and description are strings.

I've found Linq to XML great for what I've used it for, this is just a snippet. But, on the other hand I get the feeling it should or could be cleaner. The casting seems the most obvious issue in this snippet.

Any advice?

+1  A: 

I assume that you have an "Items" node as well?

You could do something like this, assuming that you are loading the document using XElement.Load()

var subset = from item in document.Elements("Item")
             where item.Element("Id").Value == itemId.ToString()
             select new PurchaseItem() {
                 Id = int.Parse(item.Element("Id").Value),
                 Name = item.Element("Name").Value,
                 Description = item.Element("Description").Value,
                 Price = int.Parse(item.Element("Price").Value)
             };

Not a lot better, but much easier to read!

Mitchel Sellers
+10  A: 

Actually it would be better IMO to cast than to call int.Parse. Here's how I would write your query:

string id = itemId.ToString(); // We don't need to convert it each time!

var subset = from item in document.Descendants("Id")
             where item.Value == id
             let parent = item.Parent
             select new PurchaseItem
             {
                 Id = (int) parent.Element("Id"),
                 Name = (string) parent.Element("Name"),
                 Description = (string) parent.Element("Description"),
                 Price = (int) parent.Element("Price")
             };
Jon Skeet
man, you are fast....
Tim Jarvis
John, any comment on why doint it this way, rather than the way I show it?
Mitchel Sellers
@Jon - How come .Value is not required? Is this called implicitly behind the scenes?
Finglas
XElement defines custom casting operators for the common data types. So when you cast an XElement, you're really calling one of those operators, which reads the value and converts it. You refrain from explicitly calling `.Value` for two reasons: One, Value is a string, and those custom operators are defined on XElement, not string. Two, casting an XElement that happens to be null will just return null, while calling .Value on a null XElement will cause an exception. Useful when filtering.
Joel Mueller
@Mitchel: I tried to keep as close to the original as possible in terms of the search itself, just streamlining it.
Jon Skeet
+1  A: 

In your example you can tidy up a little bit by finding the <Item/> element rather than the <Id/> to avoid getting the Parent each time:

var subset = from item in document.Descendants("Item")
             where item.Element("Id").Value == itemId.ToString()
             select new PurchaseItem()
                        {
                            Id = int.Parse(item.Element("Id").Value),
                            Name = item.Element("Name").Value,
                            Description = item.Element("Description").Value,
                            Price = int.Parse(item.Element("Price").Value)
                        };
GraemeF
I orginially had this, but changed it after playing with the code. So +1 for showing me it can be done.
Finglas
+1  A: 

Consider writing a new constructor for PurchaseItem that takes the XML element, so you can write:

select new PurchaseItem(item.Parent);
Jay Bazuzi