current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite

I am creating a way for users to filter results based on start and end values. An example of this would be:

?RangeColumn=DisplayId&RangeStart=640,830,670&RangeEnd=650,850,680

In this case, the api would return items where the "DisplayId" is between 640-650 or 830-850 or 670-680. My solution works but the query looks pretty bad:

{t => ((((t.DisplayId >= { Item = 640 }.Item) AndAlso (t.DisplayId <= { Item = 650 }.Item)) OrElse ((t.DisplayId >= { Item = 830 }.Item) AndAlso (t.DisplayId <= { Item = 850 }.Item))) OrElse ((t.DisplayId >= { Item = 670 }.Item) AndAlso (t.DisplayId <= { Item = 680 }.Item)))}

The user can have an unlimited number of ranges so this could quickly get crazy really fast so I'm wondering if there is a better way for me to do what I am trying to do. Here are the methods that are relevant to the problem:

private static IQueryable<T> FilterOnRange<T>(Type entityType, RangeProperties rangeProperties, IQueryable<T> query)
    {
        var startCount = rangeProperties.RangeStart.Count();
        var endCount = rangeProperties.RangeEnd.Count();

        if (startCount > 0 || endCount > 0)
        {
            if (startCount != endCount && (startCount > 1 || endCount > 1))
                throw new RangeFilterException();
        }
        else
            return query;

        PropertyInfo pi = GetFilterPropertyInfo(entityType, rangeProperties.RangeColumnName);

        if (pi != null)
        {
            bool propertyIsNullableType = pi.PropertyType.IsGenericType && pi.PropertyType.GetGenericTypeDefinition().Equals(typeof(Nullable<>));

            bool isInteger = pi.PropertyType.Equals(typeof(int)) || (propertyIsNullableType && Nullable.GetUnderlyingType(pi.PropertyType).Equals(typeof(int)));
            bool isDateTimeOffset = pi.PropertyType.Equals(typeof(DateTimeOffset)) || (propertyIsNullableType && Nullable.GetUnderlyingType(pi.PropertyType).Equals(typeof(DateTimeOffset)));

            var maxCount = Math.Max(startCount, endCount);
            ParameterExpression argParam = Expression.Parameter(typeof(T), "t");
            if (isInteger)
            {
                if (maxCount == 1)
                {
                    string start = startCount > 0 ? rangeProperties.RangeStart.Single() : null;
                    string end = endCount > 0 ? rangeProperties.RangeEnd.Single() : null;
                    var lambda = RangeInt<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);
                    query = query.Where(lambda);
                }
                else
                {
                    Expression<Func<T, bool>> outerLambda = null;
                    for (int i = 0; i < maxCount; i++)
                    {
                        string start = rangeProperties.RangeStart[i];
                        string end = rangeProperties.RangeEnd[i];
                        var lambda1 = RangeInt<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);

                        Expression<Func<T, bool>> tempLambda = null;
                        i++;
                        if (i >= maxCount)
                        {
                            tempLambda = lambda1;
                        }
                        else
                        {
                            start = rangeProperties.RangeStart[i];
                            end = rangeProperties.RangeEnd[i];
                            var lambda2 = RangeInt<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);

                            var body = Expression.OrElse(lambda1.Body, lambda2.Body);
                            tempLambda = Expression.Lambda<Func<T, bool>>(body, lambda1.Parameters[0]);
                        }

                        if (outerLambda == null)
                            outerLambda = tempLambda;
                        else
                        {
                            var body = Expression.OrElse(outerLambda.Body, tempLambda.Body);
                            outerLambda = Expression.Lambda<Func<T, bool>>(body, outerLambda.Parameters[0]);
                        }
                    }

                    query = query.Where(outerLambda);
                }
            }
            else if (isDateTimeOffset)
            {
                if (maxCount == 1)
                {
                    string start = startCount > 0 ? rangeProperties.RangeStart.Single() : null;
                    string end = endCount > 0 ? rangeProperties.RangeEnd.Single() : null;
                    var lambda = RangeDateTime<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);
                    query = query.Where(lambda);
                }
                else
                {
                    Expression<Func<T, bool>> outerLambda = null;
                    for (int i = 0; i < maxCount; i++)
                    {
                        string start = rangeProperties.RangeStart[i];
                        string end = rangeProperties.RangeEnd[i];
                        var lambda1 = RangeDateTime<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);

                        Expression<Func<T, bool>> tempLambda = null;
                        i++;
                        if (i >= maxCount)
                        {
                            tempLambda = lambda1;
                        }
                        else
                        {
                            start = rangeProperties.RangeStart[i];
                            end = rangeProperties.RangeEnd[i];
                            var lambda2 = RangeDateTime<T>(argParam, start, end, rangeProperties.RangeColumnName, propertyIsNullableType);

                            var body = Expression.OrElse(lambda1.Body, lambda2.Body);
                            tempLambda = Expression.Lambda<Func<T, bool>>(body, lambda1.Parameters[0]);
                        }

                        if (outerLambda == null)
                            outerLambda = tempLambda;
                        else
                        {
                            var body = Expression.OrElse(outerLambda.Body, tempLambda.Body);
                            outerLambda = Expression.Lambda<Func<T, bool>>(body, outerLambda.Parameters[0]);
                        }
                    }

                    query = query.Where(outerLambda);
                }
            }
            else
                throw new InvalidOperationException("Only Datetime and Integer is supported for a range filter.");
        }

        return query;
    }

    private static Expression<Func<T, bool>> RangeInt<T>(ParameterExpression argParam, string start, string end, string columnName, bool propertyIsNullableType)
    {
        int? rangeStart = null;
        int tempRangeStart = int.MinValue;

        if (int.TryParse(start, out tempRangeStart))
            rangeStart = tempRangeStart;
        else
            rangeStart = int.MinValue;

        int? rangeEnd = null;
        int tempRangeEnd = int.MaxValue;

        if (int.TryParse(end, out tempRangeEnd))
            rangeEnd = tempRangeEnd;
        else
            rangeEnd = int.MaxValue;

        if (rangeEnd < rangeStart)
            throw new InvalidOperationException("RangeEnd must be greater than RangeStart.");

        Expression rangeProperty = Expression.Property(argParam, columnName);

        MemberExpression comparisonValue2 = null;
        if (propertyIsNullableType)
        {
            var item = new { Item = (int?)rangeStart };
            var comparisonValue = Expression.Constant(item);
            comparisonValue2 = Expression.MakeMemberAccess(comparisonValue, item.GetType().GetMember("Item").First());
        }
        else
        {
            var item = new { Item = (int)rangeStart };
            var comparisonValue = Expression.Constant(item);
            comparisonValue2 = Expression.MakeMemberAccess(comparisonValue, item.GetType().GetMember("Item").First());
        }
        Expression e1 = Expression.GreaterThanOrEqual(rangeProperty, comparisonValue2);

        MemberExpression comparisonValue4 = null;
        if (propertyIsNullableType)
        {
            var item2 = new { Item = (int?)rangeEnd };
            var comparisonValue3 = Expression.Constant(item2);
            comparisonValue4 = Expression.MakeMemberAccess(comparisonValue3, item2.GetType().GetMember("Item").First());
        }
        else
        {
            var item2 = new { Item = (int)rangeEnd };
            var comparisonValue3 = Expression.Constant(item2);
            comparisonValue4 = Expression.MakeMemberAccess(comparisonValue3, item2.GetType().GetMember("Item").First());
        }
        Expression e2 = Expression.LessThanOrEqual(rangeProperty, comparisonValue4);

        Expression e3 = Expression.AndAlso(e1, e2);

        return Expression.Lambda<Func<T, bool>>(e3, argParam);
    }

    private static Expression<Func<T, bool>> RangeDateTime<T>(ParameterExpression argParam, string start, string end, string columnName, bool propertyIsNullableType)
    {
        DateTimeOffset? rangeStart = null;
        DateTimeOffset tempRangeStart = DateTimeOffset.MinValue;

        if (DateTimeOffset.TryParse(start, out tempRangeStart))
            rangeStart = tempRangeStart;
        else
            rangeStart = DateTimeOffset.MinValue;

        DateTimeOffset? rangeEnd = null;
        DateTimeOffset tempRangeEnd = DateTimeOffset.MaxValue;

        if (DateTimeOffset.TryParse(end, out tempRangeEnd))
            rangeEnd = tempRangeEnd;
        else
            rangeEnd = DateTimeOffset.MaxValue;

        if (rangeEnd < rangeStart)
            throw new InvalidOperationException("RangeEnd must be greater than RangeStart.");

        Expression rangeProperty = Expression.Property(argParam, columnName);

        MemberExpression comparisonValue2 = null;
        if (propertyIsNullableType)
        {
            var item = new { Item = (DateTimeOffset?)rangeStart };
            var comparisonValue = Expression.Constant(item);
            comparisonValue2 = Expression.MakeMemberAccess(comparisonValue, item.GetType().GetMember("Item").First());
        }
        else
        {
            var item = new { Item = (DateTimeOffset)rangeStart };
            var comparisonValue = Expression.Constant(item);
            comparisonValue2 = Expression.MakeMemberAccess(comparisonValue, item.GetType().GetMember("Item").First());
        }
        Expression e1 = Expression.GreaterThanOrEqual(rangeProperty, comparisonValue2);

        MemberExpression comparisonValue4 = null;
        if (propertyIsNullableType)
        {
            var item2 = new { Item = (DateTimeOffset?)rangeEnd };
            var comparisonValue3 = Expression.Constant(item2);
            comparisonValue4 = Expression.MakeMemberAccess(comparisonValue3, item2.GetType().GetMember("Item").First());
        }
        else
        {
            var item2 = new { Item = (DateTimeOffset)rangeEnd };
            var comparisonValue3 = Expression.Constant(item2);
            comparisonValue4 = Expression.MakeMemberAccess(comparisonValue3, item2.GetType().GetMember("Item").First());
        }
        Expression e2 = Expression.LessThanOrEqual(rangeProperty, comparisonValue4);

        Expression e3 = Expression.AndAlso(e1, e2);

        return Expression.Lambda<Func<T, bool>>(e3, argParam);
    }
share|improve this question
    
Why do you care that the expression tree looks bad? – svick Feb 26 at 19:52
    
I just worry about performance. I don't really care how it looks as long as it performs the same I guess. – gmoney12 Feb 26 at 20:01
    
Then you should look at the generated SQL, not the expression tree, that's what affects performance. Also, have you measured the performance? Does it meet your performance goals? – svick Feb 26 at 20:06
up vote 2 down vote 
var item = new { Item = (int?)rangeStart };
var comparisonValue = Expression.Constant(item);
comparisonValue2 = Expression.MakeMemberAccess(comparisonValue, item.GetType().GetMember("Item").First());

Creating the anonymous object and then accessing it in the expression doesn't give you anything. You can just use the value directly:

comparisonValue = Expression.Constant(rangeStart, typeof(int?));
share|improve this answer
    
If I change the line to Expression e1 = Expression.GreaterThanOrEqual(rangeProperty, Expression.Constant(rangeStart)); It throws an error if the field is nullable – gmoney12 Feb 26 at 20:09
    
In that case, I think Expression.Constant(rangeStart, typeof(int?)) should work. – svick Feb 26 at 20:11
up vote 0 down vote

I don't think it has to be that complicated. Would something like this work for you?

I would generate expression trees manually only if the user could enter unpredictable conditions that had to be parsed first but here you know them. The only thing that is dynamic is the number of ranges so you can put them in a list and easily select.

var nums = new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 };

var ranges = new[]
{
    new range { min = 1, max = 3 },
    new range { min = 3, max = 7 },
    new range { min = 5, max = 13 },
};

var result = ranges.Select(r => nums.Where(n => r.IsInRange(n)));

class range
{   
    public int min;
    public int max;

    public bool IsInRange(int value) 
    {
        return value >= min && value <= max;
    }
}

result

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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