Filtering foreach loops with a where condition vs continue guard clauses

Question

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?

Answer 1

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.

Answer 2

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.