views:

70

answers:

2

I'm just learning LINQ and in particular LINQ to XML, and I've written up a query that works but I'm wondering if I'm doing it a bit round-a-bout. Can the code be improved?

I have an XDocument:

<SomeDocument>
    <Prop1> val1 </Prop1>
    <Prop2> val2 </Prop2>
    <Prop3> val3 </Prop3>
</SomeDocument>

But Prop1, Prop2, and Prop3 may not be there. There may be other XDocuments which I'll be parsing with the same code, that have different properties altogether. But I'm only interested in the XDocument if it has Prop1 or both Prop1 and Prop2.

var query = from n in xml.Elements()
                    where n.Name == "Prop1" || n.Name == "Prop2"
                    select new {n.Name, n.Value};

string prop1 = null;
string prop2 = null;

foreach (var n in query)
{
    if (n.Name == "Prop1") prop1 = n.Value;
    if (n.Name == "Prop2") prop2 = n.Value;
}

if (string.IsNullOrEmpty(prop1)) { //error } 
if (string.IsNullOrEmpty(prop2)) { DoMethod1(prop1); }
else { DoMethod2(prop1, prop2); }

The code after the query seems way too long to me, though I'm not sure if there's a better way of doing what I'm trying to do. Find 1 or 2 explicit Nodes, and call relevant methods depending on what (if any) nodes are found.

+2  A: 

personally I use:

        var element = xml.Element("prop1");
        if (element!= null)
        {
            //The element exists, now do things on it!
            if(string.IsNullOrEmpty(element.Value)) {DoMe(element.Value);}
        }

if you have named properties (elements) that you know, you can just name them and it will be returned. This also saves extra looping (in your code at least)

Alastair Pitts
I think your method is more appropriate to my problem.
Josh Smeaton
+2  A: 

You could probably eliminate the middle part if you break out the results into a lookup, i.e.:

string[] propNames = new[] { "Prop1", "Prop2" };
var props = (from n in xml.Elements()
             where propNames.Contains(n.Name)
             select new { n.Name, n.Value })
            .ToLookup(e => e.Name, e => e.Value);

if (props.Contains("Prop1")) { ... }
if (props.Contains("Prop2")) { ... }
// etc.

How much of an improvement this is depends on what else you're doing with this information, but it's a little cleaner at least.

Aaronaught
This is a much better LINQ solution to the one I was trying to use I think. But perhaps the answer is not to use LINQ. Thanks
Josh Smeaton