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;
    }
5
  • 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. Mar 23, 2013 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, 2013 at 17:16
  • I corrected that part. But still the problem remains as it was.
    – Tys
    Mar 23, 2013 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)? Mar 23, 2013 at 17:33
  • Is this the same issue as: stackoverflow.com/questions/13967523/…
    – Brandon
    Mar 23, 2013 at 21:17

2 Answers 2

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
5
  • 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, 2013 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, 2013 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, 2013 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, 2013 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
    – user1228
    Oct 31, 2013 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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.