23

I'm building a LINQ query dynamically with this code. It seems to work, but when i have more than one searchString in my search, (so when multiple expressions are added, i get the following error:

Variable 'p' of type referenced from scope, but it is not defined**

I guess i can only define /use p once. But, if so, i need to alter my code a bit. Can anyone point me in the right direction here?

    if (searchStrings != null)
    {
        foreach (string searchString in searchStrings)
        {
            Expression<Func<Product, bool>> containsExpression = p => p.Name.Contains(searchString);
            filterExpressions.Add(containsExpression);
        }
    }

    Func<Expression, Expression, BinaryExpression>[] operators = new Func<Expression, Expression, BinaryExpression>[] { Expression.AndAlso };
    Expression<Func<Product, bool>> filters = this.CombinePredicates<Product>(filterExpressions, operators);

    IQueryable<Product> query = cachedProductList.AsQueryable().Where(filters);

    query.Take(itemLimit).ToList();  << **error when the query executes**


    public Expression<Func<T, bool>> CombinePredicates<T>(IList<Expression<Func<T, bool>>> predicateExpressions, Func<Expression, Expression, BinaryExpression> logicalFunction)
    {
        Expression<Func<T, bool>> filter = null;

        if (predicateExpressions.Count > 0)
        {
            Expression<Func<T, bool>> firstPredicate = predicateExpressions[0];
            Expression body = firstPredicate.Body;
            for (int i = 1; i < predicateExpressions.Count; i++)
            {
                body = logicalFunction(body, predicateExpressions[i].Body);
            }
            filter = Expression.Lambda<Func<T, bool>>(body, firstPredicate.Parameters);
        }

        return filter;
    }
abatishchev
  • 98,240
  • 88
  • 296
  • 433
Tys
  • 3,592
  • 9
  • 49
  • 71
  • I don't quite get it. It seems that your `CombinePredicates` expects `n` expressions and `n-1` operators. However, in the place you invoke it, you have an array of operators which has length of `1`. I would expect an exception of going out of bounds of the array if there are more than `2` predicates to join. – Wiktor Zychla Mar 23 '13 at 17:10
  • I did see that, i took out some stuff to make my example more compact. But i'll alter my question to make that part technically correct. – Tys Mar 23 '13 at 17:16
  • I corrected that part. But still the problem remains as it was. – Tys Mar 23 '13 at 17:18
  • Does it work when you try to combine 1 predicate (the loop in CombinePredicates would not execute and the method should just return the predicate)? – Wiktor Zychla Mar 23 '13 at 17:33
  • Is this the same issue as: http://stackoverflow.com/questions/13967523/andalso-between-several-expressionfunct-bool-referenced-from-scope – Brandon Mar 23 '13 at 21:17

2 Answers2

42

Simplifying, here are several lines which you are trying to do (I use string instead Product etc, but idea is the same):

        Expression<Func<string, bool>> c1 = x => x.Contains("111");
        Expression<Func<string, bool>> c2 = y => y.Contains("222");
        var sum = Expression.AndAlso(c1.Body, c2.Body);
        var sumExpr = Expression.Lambda(sum, c1.Parameters);
        sumExpr.Compile(); // exception here

Please notice how I expanded your foreach into two expressions with x and y - this is exactly how it looks like for compiler, that are different parameters.

In other words, you are trying to do something like this:

x => x.Contains("...") && y.Contains("...");

and compiler wondering what is that 'y' variable??

To fix it, we need to use exactly the same parameter (not just name, but also reference) for all expressions. We can fix this simplified code like this:

        Expression<Func<string, bool>> c1 = x => x.Contains("111");
        Expression<Func<string, bool>> c2 = y => y.Contains("222");
        var sum = Expression.AndAlso(c1.Body, Expression.Invoke(c2, c1.Parameters[0])); // here is the magic
        var sumExpr = Expression.Lambda(sum, c1.Parameters);
        sumExpr.Compile(); //ok

So, fixing you original code would be like:

internal static class Program
{
    public class Product
    {
        public string Name;
    }

    private static void Main(string[] args)
    {
        var searchStrings = new[] { "111", "222" };
        var cachedProductList = new List<Product>
        {
            new Product{Name = "111 should not match"},
            new Product{Name = "222 should not match"},
            new Product{Name = "111 222 should match"},
        };

        var filterExpressions = new List<Expression<Func<Product, bool>>>();
        foreach (string searchString in searchStrings)
        {
            Expression<Func<Product, bool>> containsExpression = x => x.Name.Contains(searchString); // NOT GOOD
            filterExpressions.Add(containsExpression);
        }

        var filters = CombinePredicates<Product>(filterExpressions, Expression.AndAlso);

        var query = cachedProductList.AsQueryable().Where(filters);

        var list = query.Take(10).ToList();
        foreach (var product in list)
        {
            Console.WriteLine(product.Name);
        }
    }

    public static Expression<Func<T, bool>> CombinePredicates<T>(IList<Expression<Func<T, bool>>> predicateExpressions, Func<Expression, Expression, BinaryExpression> logicalFunction)
    {
        Expression<Func<T, bool>> filter = null;

        if (predicateExpressions.Count > 0)
        {
            var firstPredicate = predicateExpressions[0];
            Expression body = firstPredicate.Body;
            for (int i = 1; i < predicateExpressions.Count; i++)
            {
                body = logicalFunction(body, Expression.Invoke(predicateExpressions[i], firstPredicate.Parameters));
            }
            filter = Expression.Lambda<Func<T, bool>>(body, firstPredicate.Parameters);
        }

        return filter;
    }
}

But notice the output:

222 should not match
111 222 should match

Not something you may expect.. This is result of using searchString in foreach, which should be rewritten in the following way:

        ...
        foreach (string searchString in searchStrings)
        {
            var name = searchString;
            Expression<Func<Product, bool>> containsExpression = x => x.Name.Contains(name);
            filterExpressions.Add(containsExpression);
        }
        ...

And here is output:

111 222 should match
Lanorkin
  • 7,310
  • 2
  • 42
  • 60
  • Thanks for your elaboration. I worked on this a bit more, and now i have noticed that when my 'cachedProductList' really comes from the HttpContext.Current.Cache, from time to time again i get Variable 'x' of type referenced from scope, but it is not defined. When i dont use caching everything works fine. Do you have any idea why this is? – Tys Mar 24 '13 at 11:20
  • Do you have actual result in cache (i.e. ...smthing.ToList()) or just IEnumerable which executes every time? Looks like you have second option while you need first one.. – Lanorkin Mar 24 '13 at 12:57
  • Nope, i store the complete list of products in the cache. And after that i want to perform my search on that list. – Tys Mar 24 '13 at 14:19
  • This 'variable blah blah scope' exception means that your Expression can not be compiled. It is nothing to do with actual list itself, so I don't think caching of list may affect it somehow. You need to isolate it further, the easiest step is to `expr.Compile()` you expression before passing to `list.Where(expr)`. This should help understand where the issue lies – Lanorkin Mar 25 '13 at 10:30
  • 4
    This was my issue. Defining the same parameter in different locations, but the "name" of the parameter wasn't enough for the expression to work, I had to use the same ParameterExpression instance everywhere. +1 –  Oct 31 '13 at 14:28
1

IMHO, no need to make the list:

var filterExpressions = new List<Expression<Func<Product, bool>>>()

You may easily live with the following in Visitor class:

public class FilterConverter : IFilterConverterVisitor<Filter> {

    private LambdaExpression ConditionClausePredicate { get; set; }
    private ParameterExpression Parameter { get; set; }

    public void Visit(Filter filter) {

        if (filter == null) {
            return;
        }

        if (this.Parameter == null) {
            this.Parameter = Expression.Parameter(filter.BaseType, "x");
        }

        ConditionClausePredicate = And(filter);
    }

    public Delegate GetConditionClause() {

        if (ConditionClausePredicate != null) {

            return ConditionClausePredicate.Compile();
        }

        return null;
    }

    private LambdaExpression And(Filter filter) {

        if (filter.BaseType == null || string.IsNullOrWhiteSpace(filter.FlattenPropertyName)) {

            //Something is wrong, passing by current filter
            return ConditionClausePredicate;
        }

        var conditionType = filter.GetCondition();
        var propertyExpression = filter.BaseType.GetFlattenPropertyExpression(filter.FlattenPropertyName, this.Parameter);

        switch (conditionType) {

            case FilterCondition.Equal: {

                var matchValue = TypeDescriptor.GetConverter(propertyExpression.ReturnType).ConvertFromString(filter.Match);
                var propertyValue = Expression.Constant(matchValue, propertyExpression.ReturnType);
                var equalExpression = Expression.Equal(propertyExpression.Body, propertyValue);
                if (ConditionClausePredicate == null) {
                    ConditionClausePredicate = Expression.Lambda(equalExpression, this.Parameter);
                } else {
                    ConditionClausePredicate = Expression.Lambda(Expression.And(ConditionClausePredicate.Body, equalExpression), this.Parameter);
                }
                break;
            }
        // and so on...
    }
}

The code is not optimal, I know, I'm a beginner and a lot of everything to be implemented... But this stuff does work. The idea is to have the only ParameterExpression per Visitor class, then to construct expressions using this parameter. After, just concatenate all expressions per one LambdaExpression clause and compile to delegate, when needed.

Alexander
  • 1,152
  • 1
  • 16
  • 18