25

I've seen some programmers use this:

foreach (var item in items)
{
    if (item.Field != null)
        continue;

    if (item.State != ItemStates.Deleted)
        continue;

    // code
}

instead of where I'd normally use:

foreach (var item in items.Where(i => i.Field != null && i.State != ItemStates.Deleted))
{
    // code
}

I've even seen a combination of both. I really like the readability with 'continue', especially with more complex conditions. Is there even a difference in performance? With a database query I'm assuming there would be. What about regular lists?

Improve this question
10
  • 3
    For regular lists it sounds like micro optimization. – apocalypse Dec 7 '15 at 12:59
  • 2
    @zgnilec: ... but actually which one of the two variants is the optimized version? I have an opinion about that, of course, but from just looking at the code this is not inherently clear for everyone. – Doc Brown Dec 7 '15 at 13:46
  • 2
    Ofcourse continue will be faster. Using linq .Where you creating additional iterator. – apocalypse Dec 7 '15 at 13:50
  • 1
    @zgnilec - Good theory. Care to post an answer explaining why you think that? Both answers which currently exist say the opposite. – Bobson Dec 7 '15 at 16:50
  • 2
    ... so the bottom line is: the performance differences between the two constructs are neglectable, and readability as well as debuggability can be achieved for both. It is simply a matter of taste which one you prefer. – Doc Brown Dec 8 '15 at 5:31
65

I would regard this as an appropriate place to use command/query separation. For example:

// query
var validItems = items.Where(i => i.Field != null && i.State != ItemStates.Deleted);
// command
foreach (var item in validItems) {
    // do stuff
}

This also allows you to give a good self-documenting name to the query result. It also helps you see opportunities for refactoring, because it's much easier to refactor code that only queries data or only mutates data than mixed code that tries to do both.

When debugging, you can break before foreach to quickly check whether the contents of validItems resolve to what you expect. You don't have to step into the lambda unless you need to. If you do need to step into the lambda, then I suggest factoring it out into a separate function, then step through that instead.

Is there a difference in performance? If the query is backed by a database, then the LINQ version has the potential to run faster, because the SQL query may be more efficient. If it's LINQ to Objects, then you won't see any real performance difference. As always, profile your code and fix the bottlenecks that are actually reported, rather than trying to predict optimisations in advance.

Improve this answer
6
  • 1
    Why would an extremely large data set make a difference? Just because the minuscule cost of the lambdas would eventually add up? – BlueRaja - Danny Pflughoeft Dec 7 '15 at 18:18
  • 1
    @BlueRaja-DannyPflughoeft: Yes you're right, this example involves no additional algorithmic complexity beyond the original code. I have removed the phrase. – Christian Hayter Dec 8 '15 at 11:06
  • Doesn't this result in two iterations over the collection? Naturally, the second one is shorter, considering only valid items are within it, but you still need to do it twice, once to filter out the elements, the second time to work with the valid items. – Andy Dec 8 '15 at 20:14
  • 1
    @DavidPacker No. The IEnumerable is being driven by the foreach loop only. – Benjamin Hodgson Dec 9 '15 at 9:19
  • 2
    @DavidPacker: That's exactly what it does; most LINQ to Objects methods are implemented using iterator blocks. The example code above will iterate through the collection exactly once, executing the Where lambda and the loop body (if the lambda returns true) once per element. – Christian Hayter Dec 9 '15 at 12:40
7

Of course there is a difference in performance, .Where() results in a delegate call being made for every single item. However, I would not worry at all about performance:

  • The clock cycles used in invoking a delegate are negligible compared to the clock cycles used by the rest of the code that iterates over the collection and checks the conditions.

  • The performance penalty of invoking a delegate is of the order of a few clock cycles, and luckily, we are long past the days when we had to worry about individual clock cycles.

If for some reason performance is really important for you at the clock cycle level, then use List<Item> instead of IList<Item>, so that the compiler can make use of direct (and inlinable) calls instead of virtual calls, and so that the iterator of List<T>, which is actually a struct, does not have to be boxed. But that's really trifling stuff.

A database query is a different situation, because there is (at least in theory) a possibility of sending the filter to the RDBMS, thus greatly improving performance: only matching rows will make the trip from the RDBMS to your program. But for that I think you would have to use linq, I do not think this expression could be sent to the RDBMS as it is.

You will really see the benefits of if(x) continue; the moment you have to debug this code: Single-stepping over if()s and continues works nicely; single-stepping into the filtering delegate is a pain.

Improve this answer
4
  • That is when something is wrong and you want to look at all the items and check in the debugger which ones have Field != null, and which ones have State != null; this might be difficult to impossible with foreach ... where. – gnasher729 Dec 7 '15 at 13:27
  • Good point with debugging. Stepping into a where is not that bad in Visual Studio, but you can't rewrite lambda expressions while debugging without recompiling and this you avoid when using if(x) continue;. – Paprik Dec 7 '15 at 13:39
  • Strictly speaking, .Where only gets invoked once. What gets invoked on each iteration is the filter delegate (and MoveNext and Current on the enumerator, when they don't get optimized out) – CodesInChaos Dec 7 '15 at 21:35
  • @CodesInChaos it took me a bit of thinking to understand what you are talking about, but of course, wh00ps, you are right, strictly speaking, .Where only gets invoked once. Fixed it. – Mike Nakis Dec 7 '15 at 21:43

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