
Best Practices for Linq Enumerables and Queryables - jonwagner
http://code.jonwagner.com/2012/08/24/best-practices-for-linq-enumerables-and-queryables/
======
solutionyogi
It is a terrible article.

1\. Why would the author recommend using extension method instead of using the
'query' syntax? The main purpose behind creating the query syntax was to make
your code easier to read. Query syntax is not at all about 'pretty' or
'clever'. It's about readability. [And why call the query syntax as 'language
extension'?]

2\. Why invent terms like 'seal' LINQ queries? By default, all LINQ querie
execution is deferred. And yes, you have to understand this deferred execution
and how it affects your code. But his code example is TERRIBLE.

var allCustomers = customers; var waCustomers = allCustomers.Where (c =>
c.Region == "WA"); var waCustomerIDs = waCustomers.Select (c => c.ID);

Why copy the variable to allCustomers? And if you need Id and name, you can
write code like this:

var customerIdAndNames = from c in customers where c.Region.Equals("WA",
StringComparison.OrdinalIgnoreCase) select new { ID = c.ID, Name = c.Name };

There was absolutely no reason to create two different IEnumerables for a
scenario where you needed different properties.

3\. Again, one has to understand that result of a query is just that, a
'query'. And you don't have to call 'ToList' method to execute the query. In
fact, if you want to iterate the query results only once, it's better to use
foreach and enumerate over the query results instead of calling ToList method
as you will be not have to consume memory to store the entire list.

4\. Horrible idea to suggest that you should always return List instead of
IEnumerable. This choice should be left up to API caller in most cases.

This whole article is garbage. It is OPPOSITE of best practices in LINQ.

~~~
upthedale
Absolutely. You saved me writing the same thing.

Regarding point 1, I would add there are also things that can be done using
the query syntax that simply can't with the extension methods. For instance,
I'm not sure how you would scope things in the same way as the 'let' clause
when using extension methods.

Regarding the other points, I would only add that deferred execution and lazy
evaluation are things to be embraced, not hidden away and overriden. Should we
also not bother with the yield keyword?

Of course, there's always some occasion where you'll want to ToList your
IEnumerable. I'd argue this should be of concern to the consumer of the
IEnumerable though, not the producer.

Point 5 at least tries to allude to something useful. Given Linq's grounding
in functional programming, of course you want to try to avoid side effects.

~~~
SimonB86
Regarding the let keyword; the following code:

    
    
      var x = from post in posts
              let keywords = post.split(' ')
              ...
    

Is compiled* into:

    
    
      var x = posts
              .Select(post => new { keywords = post.split(' '), post })
              ...
    

* If you didn't know already, the compiler transforms query syntax into extension method syntax.

~~~
upthedale
Oh yes, I did actually realise that. Thank you though.

I worded it badly. I should have been clearer in that I was following on from
solutionyogi's argument about readability.

The compiler example is a bit on the ugly side, wouldn't you say? To then
access 'keywords', it becomes

    
    
      ...
      .Where(anon => anon.keywords[0] == "verybadexample")
      .Select(anon => anon.post);
    

What I should have said was that I'm not sure how you would scope things in
the same way as the 'let' clause when using extension methods _with the same
level of readability_.

~~~
andy_t
Like this:

    
    
      var thing = 
        from x in stuff
        let derp = x.herp
        select { x.name }
    

Equals this:

    
    
      var thing = stuff.Select( x => {
        var derp = x.herp;
    
        return new { x.name };
      } );
    

edit: formatting.

I think each have their place, but this absolutely enrages me:

    
    
      var things = ( from x in thingList select x ).ToList()

------
snarfy
Uhg. Always defer materializing the query until you need it. If you are using
LINQ against something like EF it changes how the SQL is generated.

Consider var query = (from customer in context.Customers select
customer).ToList().Where( c => c.Id == 3);

vs var query = (from customer in context.Customers where customer.Id == 3
select customer);

The first LINQ query results in "SELECT * FROM customer". The WHERE clause is
applied after the result set is returned. The second generates a real WHERE
clause. The result set is filter on the server before being sent to the
client.

------
jonwagner
Thanks for the feedback. I've made some updates to the article to hopefully
improve where my meaning was not as clear as I intended, and where I should
have more strongly written "it depends on your situation".

My next controversial post will be titled "Proper Spacing Around Parentheses
in C#" ;)

------
KeyBoardG
"Sealing the chain" as you say, is indeed important for scenarios such as
lookups within a loop. If you haven't .ToList()ed your lookup LINQ var, you
will be requerying with every iteration. A simple change that can slow down
code dramatically.

------
stephenfung
The article does a good job of exposing common pitfalls to those new to
deferred execution and offers some solid advice. However, I agree that it's
overly defensive to recommend ToList'ing almost every IEnumerable to avoid
having to think about it, but this seems like a decent idea for more "software
conservative" developers.

------
skittles
These are "Best Practices for Linq Enumerables and Queryables when You're Not
Really Sure How Linq Works but Still Want to Use It".

------
SwearWord
Good to see some C# on HN

