views:

188

answers:

6

I need to write a query that will perform a keyword search on a database table. The code currently looks something like this (albeit with a hard-coded set of keywords):

var keywords = new [] { "alpha", "bravo", "charlie" };
IQueryable<Story> stories = DataContext.Stories;

foreach( var keyword in keywords )
{
    stories = from story in stories where story.Name.Contains ( keyword ) );
}

return stories;

ReSharper throws a "Access to modified closure" warning for keyword inside the foreach. I understand the error, and confirm the problem when I look at the generated SQL:

SELECT [t0].[Id], [t0].[Name]
FROM [dbo].[Story] AS [t0]
WHERE (([t0].[Name] LIKE @p0))
  AND (([t0].[Name] LIKE @p1))
  AND (([t0].[Name] LIKE @p2))
-- @p0: Input NVarChar (Size = 9; Prec = 0; Scale = 0) [%charlie%]
-- @p1: Input NVarChar (Size = 9; Prec = 0; Scale = 0) [%charlie%]
-- @p2: Input NVarChar (Size = 9; Prec = 0; Scale = 0) [%charlie%]
-- Context: SqlProvider(Sql2005) Model: AttributedMetaModel Build: 3.5.30729.1

Because the keyword iterator changes during the loop, my SQL only contains a reference to the last value ("charlie").

What should I do to avoid this problem? I could probably convert the stories queryable to a list before applying each new keyword where clause, but that seems inefficient.

SOLVED

Thanks for all the answers. Ultimately I had two separate problems, both of which have been resolved:

  1. Use local variable inside foreach() loop to avoid "Access to modified closure" problem.
  2. Use PredicateBuilder in LINQKit to dynamically assemble a list of OR clauses to allow for "any"-style keyword searches.
+4  A: 

Assign the variable to a temporary within the scope of the foreach block so that you get a fresh variable each time.

foreach( var keyword in keywords )
{
    var kwd = keyword;
    stories = from story in stories where story.Name.Contains ( kwd ) );
}

Eric Lippert has a good article (or two) explaining the dangers of including the loop variable in a closure and how to avoid it.

tvanfosson
+1 for quick fingers ;) For more info, check out http://blogs.msdn.com/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx
Adam Robinson
I just added that link to my post...lol.
tvanfosson
+1  A: 

Here's a very simple way:

var keywords = new [] { "alpha", "bravo", "charlie" };
IQueryable<Story> stories = DataContext.Stories;

foreach( var keyword in keywords )
{
    string kw = keyword;
    stories = from story in stories where story.Name.Contains ( kw ) );
}

return stories;

You could also consider

var keywords = new [] { "alpha", "bravo", "charlie" };
IQueryable<Story> stories = DataContext.Stories
                                       .Where(story => keywords.All(kw => story.Name.Contains(kw));
Jason
A: 

You need to make a local copy of keyword:

foreach( var keyword in keywords )
{
    var localKeyword = keyword;
    stories = from story in stories where story.Name.Contains ( localKeyword ) );
}
bruno conde
Why the downvote?
bruno conde
A: 

I just solved part of my problem. The "Access to modified closure" is easily fixed with a locally-scoped copy of the keyword variable, like this:

var keywords = new [] { "alpha", "bravo", "charlie" };
IQueryable<Story> stories = DataContext.Stories;

foreach( var keyword in keywords )
{
    var innerKeyword = keyword;
    stories = from story in stories where story.Name.Contains ( innerKeyword ) );
}

return stories;

Unfortunately, adding multiple where clauses may not work for me. Each LINQ where expression is separated by ANDs, so only stories that satisfy all keywords are returned. I want stories with any of the keywords.

MikeWyatt
The AND/OR question has been asked before, look for articles on Dynamic SQL here or on ScottGu's blog...
Codewerks
A: 

Haven't tried it, but does something like this work?

from story in stories
where keywords.All(kw => story.Name.Contains(kw));
Steve Cooper
A: 

Edit: This is incorrect, see the comment below.

If you want to find stories that match one or more keywords instead of all keywords, as you indicate in your follow-up answer, you should use the Any operator. I believe the following query (untested) will work:

IQueryable<Story> matchingStories = 
    from story in stories
    where keywords.Any(keyword => story.Name.Contains(keyword));
Jonas H
Any doesn't appear to work. I get a NotSupportedException: "Local sequence cannot be used in LINQ to SQL implementation of query operators except the Contains() operator."
MikeWyatt
Ah, sorry, I should think before I type...You need to look into LinqKit and its PredicateBuilder. Check out this page, the second code snippet is pretty much what you want:http://www.albahari.com/nutshell/predicatebuilder.aspx
Jonas H
I tried converting my list of strings to a queryable via AsQueryable (), and that caused a stack overflow exception when I executed the query.
MikeWyatt
PredicateBuilder looks perfect. I'm downloading LINQKit.dll right now.
MikeWyatt