candland

@candland rss self
July 19th 2008

Not using Linq to it's fullest can produce the worst of both worlds

I'm parsing some XML and need to do one thing if there are a group of XML elements with the same name and another thing if just one element with a given name.

Here my first attempt, yuck!

XElement on = null;
foreach (XElement element in elements.OrderBy(x => x.Name.LocalName))
{

var current = element;
<span class="kwrd">if</span> (elements.Count(x =&gt; current.Name.LocalName.Equals(x.Name.LocalName)) &gt; 1)
{
    <span class="kwrd">if</span> (on == <span class="kwrd">null</span> || !current.Name.LocalName.Equals(on.Name.LocalName))
    {
        <span class="kwrd">yield</span> <span class="kwrd">return</span> <span class="kwrd">new</span> JProperty(current.Name.LocalName,
                                   <span class="kwrd">new</span> JArray(
                                    elements.Where(x =&gt; current.Name.LocalName.Equals(x.Name.LocalName)).Select(
                                        x =&gt; <span class="kwrd">new</span> JValue(x.Value))));
        on = current;
    }
}
<span class="kwrd">else</span>
{
    <span class="kwrd">yield</span> <span class="kwrd">return</span> FromElement(element);
}

}

That's really bad, Linq is probably making the code worse. After some refactoring...

foreach (var group in from e in elements group e by e.Name.LocalName)
    if (group.Count() > 1)
        yield return new JProperty(group.First().Name.LocalName,
            new JArray(from x in @group select new JValue(x.Value)));
    else
        yield return FromElement(group.First());

The code here is easier to read shorter and clearer. Much better.

blog comments powered by Disqus