views:

189

answers:

3

Below is a rough and simplified code example of what I am trying to do and it is not working as expected. I am using Linq to Entity in case that matters.

Basically in the first loop with the DateClause's ends up returning nothing if I have two date clauses, works great with one date clause. In the TextClause's loop it seems to ignore every text clause after the first one.

My expectation is that the clauses would be ANDed together, so that if I do a text clause and then do another they should be added and I would get a more focused results, especially with using the Contains method.

Expectations for the dates are that I would expect to be able to do a date range with this and but if there are two dates it always returns nothing, even though I know there are records with the right dates in the range.

I am sure I am doing something wrong or plain stupid, but I can't see it.

enum Operators
{
    Contains,
    DoesNotContain,
    GreaterThan,
    LessThan
}

public void DoSomething(List<DateClauses> dateClauses, List<TextClauses> textClauses)
{
    var query = from t in context.Table
       where t.Enabled = true
       select new
       {
        title = t.title
        date = t.date
       }


    foreach (DateClause clause in dateClauses)
    {
     switch (clause.Operator)
     {
      case Operator.GreaterThan:
       query = query.Where(l => l.date > clause.Date);
       break;
      case Operator.LessThan
       query = query.Where(l => l.date < clause.Date);
       break;
     }  
    }

    foreach (TextClause clause in textClauses)
    {
     switch (clause.Operator)
     {
      case Operator.Contains:
       query = query.Where(l => l.title.Contains(clause.Text));
       break;
      case Operator.DoesNotContain
       query = query.Where(l => !l.title.Contains(clause.Text));
       break;
     }
    }
}

EDIT: Updated example code to show use of enum in the process. When I extrapolated the use of the enum in to some of the very nice solutions (that otherwise would work with a bool) cause an exception shown in a comment from me in Joel's response.

I want to say I like what I have seen in responses so far, and I have learned a few new Linq tricks, I apologize for the example as I didn't think bool vs enum would matter much with Linq. I hope the changes will help find a solution that can work for me. Thanks again for the great responses to date.

A: 

I've done this before with no issues- perhaps you should try moving the .Select to the end (after the Wheres are done). Also, I use PredicateBuilder heavily for added flexibility with dynamic query stuff- right now you're getting "and" semantics for your appended wheres, where I'd guess you want "or" semantics for at least some of them. PredicateBuilder lets you do that easily (and it's free!).

nitzmahone
This is a simplified example of code, in the real code I have to use the Where clause methods, as they need to be dynamic, they are user controlled as to what is being "filtered" and what isn't and t changes at run time. The select needs to be where it is.
Rodney Foley
+2  A: 
 var query = context.Table
         .Where(t => t.Enabled 
             && textClauses.Where(c => c.Contains).All(c => t.title.Contains(c.Text) )
             && textClauses.Where(c => c.DoesNotContain).All(c => !t.title.Contains(c.Text) )
             && dateClauses.Where(c => c.GreaterThan).All(c => t.date > c.Date) )
             && dateClauses.Where(c => c.LesserThan).All(c => t.date < c.Date) )
          ).Select(t =>  new {
               title = t.title
               date = t.date
      });

The main thing here is that each of your current foreach loops can be refactored to use .All() instead. Or, rather, each if condition inside the loop can be. If you really want to you can still break out each of the where's:

var query = context.Table.Where(t => t.Enabled).Select(t =>  new {
               title = t.title
               date = t.date
      });

query = query.Where(t => textClauses.Where(c => c.Contains).All(c => t.title.Contains(c.Text) );
query = query.Where(t => textClauses.Where(c => c.DoesNotContain).All(c => !t.title.Contains(c.Text) );
query = query.Where(t => dateClauses.Where(c => c.GreaterThan).All(c => t.date > c.Date) );
query = query.Where(t => dateClauses.Where(c => c.LesserThan).All(c => t.date < c.Date) );
Joel Coehoorn
I tried the date one and it doesn't work. It compiles but gets a runtime NotSupportedException "Unable to create a constant value of type 'Closure type'. Only primitive types ('such as Int32, String, and Guid') are supported in this context." This doesn't appear to be supported by Linq to Entity.
Rodney Foley
Odd. This is definitely supported. The key word that stands out for me here from your error message is `constant`. Nowhere that I see in this code are any constants created. Is this in a function that you will call to initialize a constant?
Joel Coehoorn
The Constant would be the Enum I have in the real code I guess. In my example clause.Contains would be clause.Operator (which is a enum) and instead of If statements I have a switch. I should have made it an enum which is what it is, I didn't think it would matter and I wanted to keep the example simple I guess I may have made it too simple. What I did is I converted your (c => c.Contains) to (c => c.Operator == Operator.Contains) which would have the same results however I am learning Linq doesn't work as you would think sometimes.
Rodney Foley
+2  A: 

Just a wild guess, but could it be that changing your loops to the following has different results?

foreach (DateClause clause in dateClauses)
{
    var capturedClause = clause;
    switch (clause.Operator)
    {
            case Operator.GreaterThan:
                    query = query.Where(l => l.date > capturedClause.Date);
                    break;
            case Operator.LessThan
                    query = query.Where(l => l.date < capturedClause.Date);
                    break;
    }               
}

You're creating Where criteria capturing the loop variable, not the value of the loop variable for each iteration. So in the end, each Where will be the same, using the value for the last iteration of the loop, as the query is executed after the last iteration. By introducing a temp variable you'll capture the variable for each iteration.

See also Eric Lippert's blog about this subject.

Ruben
Wow, something so simple fixed the issue allowing me to use the loops. Thanks Ruben. Its odd how the foreach is different for predicates, thanks for the info and the link to the blog post.
Rodney Foley