If statement or Where extension with a For Each loop?

Question

I was wondering which of the two code samples would be more efficient (or is the difference between the two negligible)?

For Each apple in AppleController.GetRedApples().Where(Function(a) PriceController.OnSale(a))
   'do something
Next

or

For Each apple in AppleController.GetRedApples()
   If PriceController.OnSale(apple) Then
      'do something
   End If
Next

Thanks!

Answer 1

I would personally separate it out:

Dim applesOnSale = AppleController.GetRedApples() _
                                  .Where(Function(a) PriceController.OnSale(a))
For Each apple in applesOnSale
   'do something
Next

In a full IDE the first part could be a single line - or you could use a VB query expression like this:

Dim applesOnSale = From apple in AppleController.GetRedApples() _
                   Where PriceController.OnSale(apple)

For Each apple in applesOnSale
   'do something
Next

This separates the "what items you're interested in" from "what you want to do with the items" which I personally find useful in terms of readability.

Note that in some situations the Where may not work, however - if GetRedApples returns a table from a LINQ to SQL DataContext, for example, it will try to translate the Where clause into SQL - which won't work.

Assuming you're using LINQ to Objects though, the assignment won't actually do any looping - it will just set up the query. The "where" clause will only be evaluated for each item as you loop through the query.

Answer 2

The difference is negligible, and the second code is much more readable, so you should probably prefer that one

Answer 3

Consider this discussion on foreach vs linq/lambda (also answer by Jon Skeet ;-) and this one if you want to delve deeper into understanding LINQ / Lambda efficiency and performance.

Answer 4

The performance should be about the same, therefore I would go for the more readable approach:

var applesOnSale = from apple in AppleController.GetRedApples()
                   where PriceController.OnSale(apple)
                   select apple;

foreach(var apple in applesOnSale)
{
    //do stuff
}

This is the same as your second option but is a touch more readable.