3

I have a predicate handler that tests almost 200 cases, and each test involves five possible comparisons. I want to streamline this code, but am hitting a wall with how to express this syntactically.

public static Expression<Func<OADataConsolidated, bool>> Match(DOTSearchFilter filters)
{
    var predicate = (filters.OrFilters.Count > 0) ? PredicateBuilder.False<OADataConsolidated>() : PredicateBuilder.True<OADataConsolidated>();

    foreach (DOTFilter f in filters.AndFilters)
    {
        int value = -1;
        int.TryParse(f.TextValue, out value);
        switch (f.Type)
        {
            case DOTFilter.FilterType.SCO:
                switch (f.IdValue)
                {
                    case 4: // GED: Reasoning
                        switch (f.Comp)
                        {
                            case DOTFilter.Comparitor.LessThan:
                                predicate = predicate.And(p => p.ajblGEDR_Mean < value);
                                break;
                            case DOTFilter.Comparitor.EqualOrLess:
                                predicate = predicate.And(p => p.ajblGEDR_Mean <= value);
                                break;
                            case DOTFilter.Comparitor.EqualTo:
                                predicate = predicate.And(p => p.ajblGEDR_Mean == value);
                                break;
                            case DOTFilter.Comparitor.EqualOrGreater:
                                predicate = predicate.And(p => p.ajblGEDR_Mean >= value);
                                break;
                            case DOTFilter.Comparitor.GreaterThan:
                                predicate = predicate.And(p => p.ajblGEDR_Mean > value);
                                break;
                        }
                        break;
                    case 5: // GED: Mathematics
                        switch (f.Comp)
                        {
                            case DOTFilter.Comparitor.LessThan:
                                predicate = predicate.And(p => p.ajblGEDM < value);
                                break;
                            case DOTFilter.Comparitor.EqualOrLess:
                                predicate = predicate.And(p => p.ajblGEDM <= value);
                                break;
                            case DOTFilter.Comparitor.EqualTo:
                                predicate = predicate.And(p => p.ajblGEDM == value);
                                break;
                            case DOTFilter.Comparitor.EqualOrGreater:
                                predicate = predicate.And(p => p.ajblGEDM >= value);
                                break;
                            case DOTFilter.Comparitor.GreaterThan:
                                predicate = predicate.And(p => p.ajblGEDM > value);
                                break;
                        }
                        break;

The above switch statement is repeated almost 200 times and the only thing different in each case is the field name being checked. I want to reduce this code as much as possible.

Bob Jones
  • 2,049
  • 5
  • 32
  • 60
  • 2
    I hope you're not using -1 as a sentinel for `value` -- TryParse is guaranteed to set the value no matter what; if it returns false, it will have set `value` to 0. In other words, `value` will be equal to -1 only if `f.TextValue` parses to -1. And why aren't you checking the return value of `TryParse`? – phoog Jul 30 '12 at 22:42

1 Answers1

2

You can build the expression dynamically like that:

string propertyName = GetPropertyName(f);
ExpressionType comp = GetComparisonType(f);
ParameterExpression p = Expression.Parameter(typeof(OADataConsolidated));
Expression<Func<OADataConsolidated, bool>> expr =
    Expression.Lambda<Func<OADataConsolidated, bool>>(
        Expression.MakeBinary(
            comp,
            Expression.Property(p, propertyName),
            Expression.Constant((double)value)),
        p);

predicate = predicate.And(expr);


...

static string GetPropertyName(DOTFilter filter)
{
    switch(filter.IdValue)
    {
        case 4: // GED: Reasoning
            propertyName = "ajblGEDR_Mean";
            break;
        case 5: // GED: Mathematics
            propertyName = "ajblGEDM";
            break;
        ...
        default:
            throw new ArgumentException("Unknown Id value");
    }
}

static ExpressionType GetComparisonType(DOTFilter filter)
{
    switch (filter.Comp)
    {
        case DOTFilter.Comparitor.LessThan:
            return ExpressionType.LessThan;
        case DOTFilter.Comparitor.EqualOrLess:
            return ExpressionType.LessThanOrEqual;
        case DOTFilter.Comparitor.EqualTo:
            return ExpressionType.Equal;
        case DOTFilter.Comparitor.EqualOrGreater:
            return ExpressionType.GreaterThanOrEqual;
        case DOTFilter.Comparitor.GreaterThan:
            return ExpressionType.GreaterThan;
        default:
            throw new ArgumentException("Unknown Comp value");
    }
}

The switches are still there, but they're not repeated.

Thomas Levesque
  • 286,951
  • 70
  • 623
  • 758
  • Thomas - thanks. Looks very tight, but I can't get it to compile because GetComparisonType is an enum but the function returns a string. Fixed that by making it an Expression function, but cannot compile the line "predicate = predicate.And(expr). Compiler says: "cannot convert from 'System.Linq.Expressions.Expression' to 'System.Linq.Expressions.Expression>". I think you are close, though... – Bob Jones Jul 30 '12 at 23:59
  • @BobJones, I didn't actually try this code, so there might be a few mistakes. I made a few changes in my answer to fix the problems you mentioned. – Thomas Levesque Jul 31 '12 at 00:24
  • thanks - I got it to compile, but it threw an InvalidOperatorexception: "The binary operator LessThan is not defined for the types 'System.Nullable`1[System.Double]' and 'System.Int32'." - the fields being compared are floating point, if this makes a difference. And I would love to understand your code - it's beyond my "code-grade". Can you point me to where I can learn more about it? – Bob Jones Aug 01 '12 at 21:12
  • @BobJones, the problem is that a Double is not directly comparable to an Int32; the Int32 must be converted to Double before. The C# compiler does it implicitly, but if you generate the expression manually you need to insert the conversion explicitly with [Expression.Convert](http://msdn.microsoft.com/en-us/library/bb292051.aspx) – Thomas Levesque Aug 01 '12 at 23:43
  • thank you (again) for your help. If you could annotate your example to show where you use the ExpressionConvert, that would help me a bunch. And I will flag your answer as correct. – Bob Jones Aug 03 '12 at 15:19
  • @BobJones, actually I just realized you don't even need `Expression.Convert`, you can just convert `value` to double directly (I updated the code) – Thomas Levesque Aug 03 '12 at 16:35