Creating dynamic expression for entity framework

Question

I've created a generic expression builder that builds up a predicate based on collection of conditions. I pass the predicate to a generic method in the repository. I think that expression builder works fine and creates the desired predicate although the SQL script generated by Entity Framework is not as I expected. I've read many questions and article regarding dynamic query or LinqKit and expression builder to this issue and the most relevant was this comment. I really appreciate if you could look over what I have done and let me know if I made any mistake?

Here is the code for ExpressionBuilder class:

public static class ExpressionBuilder
{
    private static MethodInfo containsMethod = typeof(string).GetMethod("Contains");
    private static MethodInfo startsWithMethod = typeof(string).GetMethod("StartsWith", new Type[] { typeof(string) });
    private static MethodInfo endsWithMethod = typeof(string).GetMethod("EndsWith", new Type[] { typeof(string) });

    public static Expression<Func<T, bool>> GetExpression<T>(IList<ExpressionModel> filters)
    {
        if (filters == null)
            return null;

        IList<ExpressionModel> nullFreeCollection = filters.OfType<ExpressionModel>().ToList();

        if (nullFreeCollection.Count == 0)
            return null;

        ParameterExpression param = Expression.Parameter(typeof(T), "item");
        Expression exp = null;

        if (nullFreeCollection.Count == 1)
            exp = GetExpression<T>(param, nullFreeCollection[0]);
        else if (nullFreeCollection.Count == 2)
            exp = GetExpression<T>(param, nullFreeCollection[0], nullFreeCollection[1]);
        else
        {
            while (nullFreeCollection.Count > 0)
            {
                var f1 = nullFreeCollection[0];
                var f2 = nullFreeCollection[1];

                if (exp == null)
                    exp = GetExpression<T>(param, nullFreeCollection[0], nullFreeCollection[1]);
                else
                    exp = Expression.AndAlso(exp, GetExpression<T>(param, nullFreeCollection[0], nullFreeCollection[1]));

                nullFreeCollection.Remove(f1);
                nullFreeCollection.Remove(f2);

                if (nullFreeCollection.Count == 1)
                {
                    exp = Expression.AndAlso(exp, GetExpression<T>(param, nullFreeCollection[0]));
                    nullFreeCollection.RemoveAt(0);
                }
            }
        }

        return Expression.Lambda<Func<T, bool>>(exp, param);
    }

    private static Expression GetExpression<T>(ParameterExpression param, ExpressionModel filter)
    {
        MemberExpression member = Expression.Property(param, filter.PropertyName);
        ConstantExpression constant = Expression.Constant(filter.Value);

        switch (filter.Operator)
        {
            case ExpressionOperators.Equals:
                return Expression.Equal(member, constant);
            case ExpressionOperators.GreaterThan:
                return Expression.GreaterThan(member, constant);
            case ExpressionOperators.LessThan:
                return Expression.LessThan(member, constant);
            case ExpressionOperators.GreaterThanOrEqual:
                return Expression.GreaterThanOrEqual(member, constant);
            case ExpressionOperators.LessThanOrEqual:
                return Expression.LessThanOrEqual(member, constant);
            case ExpressionOperators.Contains:
                return Expression.Call(member, containsMethod, constant);
            case ExpressionOperators.StartsWith:
                return Expression.Call(member, startsWithMethod, constant);
            case ExpressionOperators.EndsWith:
                return Expression.Call(member, endsWithMethod, constant);
        }

        return null;
    }

    private static BinaryExpression GetExpression<T>(ParameterExpression param, ExpressionModel filter1, ExpressionModel filter2)
    {
        Expression bin1 = GetExpression<T>(param, filter1);
        Expression bin2 = GetExpression<T>(param, filter2);

        return Expression.AndAlso(bin1, bin2);
    }

    public enum ExpressionOperators
    {
        Equals,
        GreaterThan,
        LessThan,
        GreaterThanOrEqual,
        LessThanOrEqual,
        Contains,
        StartsWith,
        EndsWith
    }
}

And here is the generic repository method:

    public IEnumerable<TEntity> RetrieveCollectionAsync(Expression<Func<TEntity, bool>> predicate)
    {
        try
        {
            return DataContext.Set<TEntity>().Where(predicate);
        }
        catch (Exception ex)
        {
            Logger.Error(ex);
            throw ex;
        }
    }

And generated script by Entity Framework for Sql (I expect a select query with a few where clause):

SELECT 
    CAST(NULL AS uniqueidentifier) AS [C1], 
    CAST(NULL AS uniqueidentifier) AS [C2], 
    CAST(NULL AS varchar(1)) AS [C3], 
    CAST(NULL AS uniqueidentifier) AS [C4], 
    CAST(NULL AS uniqueidentifier) AS [C5], 
    CAST(NULL AS uniqueidentifier) AS [C6], 
    CAST(NULL AS datetime2) AS [C7], 
    CAST(NULL AS datetime2) AS [C8], 
    CAST(NULL AS varchar(1)) AS [C9], 
    CAST(NULL AS uniqueidentifier) AS [C10], 
    CAST(NULL AS varchar(1)) AS [C11], 
    CAST(NULL AS uniqueidentifier) AS [C12], 
    CAST(NULL AS uniqueidentifier) AS [C13], 
    CAST(NULL AS uniqueidentifier) AS [C14], 
    CAST(NULL AS uniqueidentifier) AS [C15], 
    CAST(NULL AS datetime2) AS [C16], 
    CAST(NULL AS varchar(1)) AS [C17], 
    CAST(NULL AS datetime2) AS [C18], 
    CAST(NULL AS varchar(1)) AS [C19], 
    CAST(NULL AS tinyint) AS [C20]
    FROM  ( SELECT 1 AS X ) AS [SingleRowTable1]
    WHERE 1 = 0

I am using

• EntityFramework 6.0
• .Net Framework 4.6
• ASP.NET MVC 5

Update the model for expression:

public class ExpressionModel
{
    public string PropertyName { get; set; }
    public ExpressionOperators Operator { get; set; }
    public object Value { get; set; }
}

Another missing part is a generic mapper that maps a given search criteria to a new ExpressionModel which I believe it is not relevant to this problem.

Answer 1

As I mentioned in the comments, the implementation is too overcomplicated.

First, this method

private static BinaryExpression GetExpression<T>(ParameterExpression param, ExpressionModel filter1, ExpressionModel filter2)

and the whole logic for checking filters count, removing processed items, etc. is redundant. AND conditions can easily be chained like this

((Condition1 AND Condition2) AND Condition3) AND Condition4 ...

So just remove that function.

Second, this function

private static Expression GetExpression<T>(ParameterExpression param, ExpressionModel filter)

is poorly named and does not need a generic T because it's not used inside.

Instead, change the signature to

private static Expression MakePredicate(ParameterExpression item, ExpressionModel filter)
{
    // implementation (same as posted)
}

Finally, the public method is simple as that:

public static Expression<Func<T, bool>> MakePredicate<T>(IEnumerable<ExpressionModel> filters)
{
    if (filters == null) return null;
    filters = filters.Where(filter => filter != null);
    if (!filters.Any()) return null;
    var item = Expression.Parameter(typeof(T), "item");
    var body = filters.Select(filter => MakePredicate(item, filter)).Aggregate(Expression.AndAlso);
    var predicate = Expression.Lambda<Func<T, bool>>(body, item);
    return predicate;
}

P.S. And don't forget to do null check in the usage:

// should not be called Async
public IEnumerable<TEntity> RetrieveCollectionAsync(Expression<Func<TEntity, bool>> predicate)
{
    try
    {
        var query = DataContext.Set<TEntity>().AsQueryable();
        if (predicate != null)
            query = query.Where(predicate);
        return query;
    }
    catch (Exception ex)
    {
        Logger.Error(ex);
        throw ex; // should be: throw;
    }
}