tags:

views:

103

answers:

2

I'm new to LINQ, and don't want to over do it and make this code hard to maintain.

What do you think, is this LINQ Query too long?

IList<ListViewItem> data = runAnalysis.PassAnalyses.Cast<PassAnalysis>().
  Select(passAnalysis => passAnalysis.GetValue(PeakStatistics.PeakStatisticsProperty)).
  SelectMany(peakStatistics => peakStatistics.Statistics.
    Where(statisticsBase => statisticsBase.Name == statisticType).
    Select(statisticsBase => new ListViewItem {Content = statisticsBase})).ToList();
+6  A: 

IMO the query is fine, although it looks somewhat dense. You could reformat it a bit to be more spacey:

var query = from analysis in runAnalysis.PassAnalyses.Cast<PassAnalysis>()
            let value = analysis.GetValue(PeakStatistics.PeakStatisticsProperty)
            from statistic in value.Statistics
            where statistic.Name == statisticType
            select new ListViewItem { Content = statistic };

IList<ListViewItem> data = query.ToList();
dtb
+1 the sql syntax makes it WAY easier to read...
CrazyJugglerDrummer
For some reason I like the the method chaining way better.
ChaosPandion
@ChaosPandion: That's probably why we have both in C# :-)
dtb
+10  A: 

I would say it's complicated due to nesting - but doesn't really need to be. Here's an example of a query which will give the same results (I think), but is simpler...

IList<ListViewItem> data = runAnalysis.PassAnalyses
      .Cast<PassAnalysis>()
      .Select(pass => pass.GetValue(PeakStatistics.PeakStatisticsProperty))
      .SelectMany(peakStats => peakStats.Statistics)
      .Where(statisticsBase => statisticsBase.Name == statisticType)
      .Select(statisticsBase => new ListViewItem {Content = statisticsBase})
      .ToList();

Without the nesting, it's easy to see how the transformation goes:

  • Start with PassAnalyses
  • Cast each item to a PassAnalysis
  • Select the PeakStatistics
  • From each element, select all the Statistics within it, and flatten this sequence
  • Filter any statistic with the wrong name
  • Convert each result into a ListViewItem
  • Convert the whole sequence into a list

At this point, it's easy to convert it into a query expression:

IList<ListViewItem> data =
   (from PassAnalysis pass in runAnalysis.PassAnalyses
    from statsBase in pass.GetValue(PeakStatistics.PeakStatisticsProperty)
                          .Statistics
    where statsBase.Name == statisticType
    select new ListViewItem { Content = statsBase })
   .ToList();

Note that I've elided the first Select and the SelectMany; you could use a let clause if you wanted. Also I've used an explicit type for the pass range variable, to make the compiler generate the Cast<PassAnalysis >() call.

This is slightly different to the original version, as it will use a different form of SelectMany which propagates the original pass value too, but the results will be the same.

Calling ToList() at the end is somewhat ugly as there's no query expression syntax for it... but you could use an intermediate variable for that:

var query = ...;
IList<ListViewItem> data = query.ToList();
Jon Skeet
This is great, very informative, thank you.
mkocubinski