views:

133

answers:

1

We have a project using LINQ to SQL, for which I need to rewrite a couple of search pages to allow the client to select whether they wish to perform an and or an or search.

I though about redoing the LINQ queries using PredicateBuilder and have got this working pretty well I think. I effectively have a class containing my predicates, e.g.:

internal static Expression<Func<Job, bool>> Description(string term)
{
    return p => p.Description.Contains(term);
}

To perform the search i'm doing this (some code omitted for brevity):

public Expression<Func<Job, bool>> ToLinqExpression()
{
    var predicates = new List<Expression<Func<Job, bool>>>();
    // build up predicates here

    if (SearchType == SearchType.And)
    {
        query = PredicateBuilder.True<Job>();
    }
    else
    {
        query = PredicateBuilder.False<Job>();
    }

    foreach (var predicate in predicates)
    {
        if (SearchType == SearchType.And)
        {
            query = query.And(predicate);
        }
        else
        {
            query = query.Or(predicate);
        }
    }
    return query;
}

While i'm reasonably happy with this, I have two concerns:

  1. The if/else blocks that evaluate a SearchType property feel like they could be a potential code smell.
  2. The client is now insisting on being able to perform 'and not' / 'or not' searches.

To address point 2, I think I could do this by simply rewriting my expressions, e.g.:

internal static Expression<Func<Job, bool>> Description(string term, bool invert)
{
    if (invert)
    {
        return p => !p.Description.Contains(term);
    }
    else
    {
        return p => p.Description.Contains(term);
    }
}

However this feels like a bit of a kludge, which usually means there's a better solution out there. Can anyone recommend how this could be improved? I'm aware of dynamic LINQ, but I don't really want to lose LINQ's strong typing.

+2  A: 

If you are looking for less lines you can replace the if/else with ternary operator:

query = SearchType == SearchType.And ? PredicateBuilder.True<Job>() : PredicateBuilder.False<Job>();

   foreach (var predicate in predicates)
   {
        query = SearchType == SearchType.And ? query.And(predicate) : query.Or(predicate);
   }

for the 'and not' / 'or not' part the ! operator should do the trick.

PD: Did you test the foreach part is correctly setting the predicates?, as far as i remember you are building the expression that will be executed at later point in time, so you may have a literal reference just to the last set predicate in the final iteration, and that's why you must use a temp variable to save the value of each iteration.

EDIT: If you want to negate a expression programmatic, that's a tricky one, you can try something like:

internal static Expression<Func<Job, bool>> Description(string term, bool invert)
        {
           return NegateExp<Func<Job, bool>>(p => p.Description.Contains(term), invert);
        }

And the NegateExp method will be something like:

public static Expression<TDelegate> NegateExp<TDelegate>(Expression<TDelegate> expression, bool inverse)
        {
            if (inverse)
            {
                return Expression.Lambda<TDelegate>(Expression.Not(expression.Body), expression.Parameters);
            }
            return expression;
        }

You can take a look at this question for more examples http://stackoverflow.com/questions/2166591/is-there-any-way-to-negate-a-predicate

Omar
I've been profiling the generated SQL, and it all looks ok. Fair comment about using ternary operators although i'm more concerned about the duplication in my updated predicates.
richeym
using Expression.Not with a delegate should be usefull here. Added a little example above.
Omar
That's an interesting one, will give it a try tomorrow. It also occurred to me that I could simply XOR the expression with the invert parameter - It works but it generates some pretty inefficient SQL.
richeym
Works perfectly *and* generates clean SQL. Thanks!
richeym