Linq filtering results with multiple Where clauses

Question

I am trying to use EF 5 to apply multiple search criteria to a result set (in this case, for a library catalog search). Here is the relevant code:

public IQueryable<LibraryResource> GetSearchResults(string SearchCriteria, int? limit = null)
    {
        List<string> criteria = SearchCriteria.Split(new char[] { ' ' }, StringSplitOptions.RemoveEmptyEntries).ToList();
        IQueryable<LibraryResource> allResults = context.LibraryResources.Include("Type").Where(r => r.AuditInfo.DeletedAt == null);
        foreach (string criterion in criteria)
        {
            allResults = allResults.Where(r => (r.Title.Contains(criterion) || r.Keywords.Contains(criterion) || r.Author.Contains(criterion) || r.Comments.Contains(criterion)));
        }
        allResults = allResults.OrderBy(r => r.Title);
        if (limit.HasValue) allResults = allResults.Take(limit.Value);
        return allResults;
    }

Sample SearchCriteria = "history era"

For some reason, only the last criterion gets applied. For instance, in the sample above, all the books with "era" in the title, author, keywords and comments are returned, without also filtering by "history". I stepped through the code, and the loop executes twice, with the appropriate criterion each time. Can you see something I can't? Thanks!

Also, you may want to short circuit the or's – colin-higgins Feb 04 '13 at 21:06 — colin-higgins, Feb 04 '13 at 21:06

score 5 · Accepted Answer · edited May 23 '17 at 12:18

You have fallen victim to modifying the value of a closed-over variable.

Change the code to this:

foreach (string criterion in criteria)
{
    var crit = criterion;
    allResults = allResults.Where(/* use crit here, not criterion */);
}

The problem here is that while you are building up the query your filtering expressions close over the variable criterion, in effect pulling it in scope at the point where the query is evaluated. However, at that time criterion will only have one value (the last one it happened to loop over) so all but the last of your filters will in fact be turned into duplicates of the last one.

Creating a local copy of criterion and referencing that inside the expressions corrects the problem because crit is a different local variable each time, with lifetime that does not extend from one iteration of the loop to the next one.

For more details you might want to read Is there a reason for C#'s reuse of the variable in a foreach?, where it is also mentioned that C# 5.0 will take a breaking change that applies to this scenario: the lifetime of the loop variable criterion is going to change, making this code work correctly without an extra local.

You're a star, Jon. That was it. – Tom Sweeney Feb 04 '13 at 21:22 — Tom Sweeney, Feb 04 '13 at 21:22

Linq filtering results with multiple Where clauses

1 Answers1