13
\$\begingroup\$

I don't like the fact that I am selecting all the points, even though I only care about the count of them.

Which of the from parts should appear first, if it even matters?

var count = (from type in ValidTypes()
    from point in GetData(type)
    where point.Radius > referencePoint.Radius
        && point.Theta > referencePoint.Theta
    select point).Count()

ValidTypes() returns a few types, about 5.

GetData(type) may return many points, possibly 100,000.

This query is counting the number of points that have both a larger theta and a larger radius than a given point.

Is there a way to write this faster or more readable?

\$\endgroup\$
  • 1
    \$\begingroup\$ Is this a Linq to sql query or linq to objects? \$\endgroup\$ – Sean Lynch Jan 21 '11 at 19:28
  • \$\begingroup\$ This is linq to objects. \$\endgroup\$ – mjcopple Jan 21 '11 at 19:56
  • 1
    \$\begingroup\$ As has been said already, your code is fine. I'm not sure what you mean by "selecting" when you say "I am selecting all the points"? That you're iterating over all of them? You can't count them without iterating over them, so that's ok. Or do you think that your linq query creates a temporary array which you then call Count on? It doesn't. \$\endgroup\$ – sepp2k Jan 22 '11 at 12:08
9
\$\begingroup\$

First, your query is written fine. Its structure is not a concern; the order of the froms is clearly correct, the select is appropriate. Where you could look for improvement that could change the query (code and possibly performance) would be in the GetData method. If, for example, GetData is getting data from an external, queryable source, then you may want to offload the filtering to it rather than getting all of the data points and filtering it inside your application, particularly since you have so many points of data (potentially). So maybe your query could instead be

(from type in ValidTypes()    
 select GetData(type, referencePoint).Count())
.Sum();

// or same meaning, different phrasing

(from type in ValidTypes()
from point in GetData(type, referencePoint)
select point).Count();
\$\endgroup\$
7
\$\begingroup\$

Why are you doing:

Point.Radius > referencePoint.Radius && point.Theta > referencePoint.Theta

Seems this probably should be a function like IsCorrectReferencePoint or whatnot. That will both make your code more readable and more understandable since you can simply look at the function name to see what it does.

\$\endgroup\$
  • \$\begingroup\$ Make sure the function is a proper Predicate<> so it can be used in the overload of .Count() as in Martin's answer. \$\endgroup\$ – Snowbody Jul 8 '14 at 2:21
3
\$\begingroup\$

This is an old question, but maybe worth reminding for people that .Count() has a useful overload. Re-writing the query in fluent syntax could let you write:

int count = ValidTypes()
    .SelectMany(type => GetData(type))
    .Count(point => point.Radius > referencePoint.Radius 
                 && point.Theta > referencePoint.Theta);
\$\endgroup\$
2
\$\begingroup\$

I don't like writing a statement that combines both "fluent" and "expression" syntax for LINQ. (See SO: Which LINQ syntax do you prefer? Fluent or Query Expression)

I also would choose multiple where clauses over a single where with &&. So, either:

var points = from type in ValidTypes()
    from point in GetData(type)
    where point.Theta > referencePoint.Theta
    where point.Radius > referencePoint.Radius
    select point;

var count = points.Count();

or

var count = ValidTypes
    .Select(type => type.GetData())
    .Where(point.Theta > referencePoint.Theta)
    .Where(point.Radius > referencePoint.Radius)
    .Count();

I'd look at the code before & after, and see if one of these lends itself to further refactoring.

\$\endgroup\$
  • 1
    \$\begingroup\$ ugh, Your fluent version is syntactically incorrect and should be using SelectMany or else you'll get the wrong result. Also, see <a href="stackoverflow.com/questions/664683/…> regarding multiple where vs && \$\endgroup\$ – Marty Neal Mar 30 '12 at 22:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.