Greg Tarr
Greg Tarr

Reputation: 506

Evaluating boolean expression in C#

I have written the code below to evaluate a boolean expression. The expression is coded in the form of objects.

It's one of those moments when I look at the code and think: I'm sure there's a better way to code that, using less boolean variables but can't see the right way to go. Any help? Unit tests have been written and are passing for a variety of inputs.

if (tree == null || !tree.IsActive || tree.FilterNodes == null)
{
     return false;
}
var result = false;
foreach (var filter in tree.FilterNodes.Where(a => a.IsActive && a.ConditionNodes != null))
{
     var tempBool = false;
     foreach (var condition in filter.ConditionNodes.Where(a => a.IsActive))
     {
          if (!string.IsNullOrWhiteSpace(condition.FieldName) && values.ContainsKey(condition.FieldName))
          {
               var value = values[condition.FieldName];
               if (filter.LogicalOperator == LogicalOperator.Or && ApplyCondition(condition.ConditionOperator, value, condition.FieldValue))
               {
                   tempBool = true;
                   break;
               }
               else if (filter.LogicalOperator == LogicalOperator.And)
               {
                   tempBool = ApplyCondition(condition.ConditionOperator, value, condition.FieldValue);
                   if (!tempBool)
                   {
                        break;
                   }
               }
               else
               {
                   tempBool = false;
               }
           }
           else if (!string.IsNullOrWhiteSpace(condition.FieldName) && filter.LogicalOperator == LogicalOperator.And)
           {
                tempBool = false;
           }
     }
     result = tempBool;
     if (!result)
     {
          break;
     }
}
return result;

Upvotes: 0

Views: 2543

Answers (3)

Francesco Bonizzi
Francesco Bonizzi

Reputation: 5302

A more formal way to code a boolean interpreter is considering a boolean expression as generated by a formal grammar and writing a parser and an interpreter for it. The interpreter could be implemented as an abstract syntax tree.

I made an open source library to achieve this, if you want you can take a look on GitHub.

Upvotes: 0

Romano Zumbé
Romano Zumbé

Reputation: 8079

You could set tempBool = false first thing in the loop and leave out the else and last else if:

 foreach (var condition in filter.ConditionNodes.Where(a => a.IsActive))
 {
      tempBool = false;
      if (!string.IsNullOrWhiteSpace(condition.FieldName) && values.ContainsKey(condition.FieldName))
      {
           var value = values[condition.FieldName];
           if (filter.LogicalOperator == LogicalOperator.Or && ApplyCondition(condition.ConditionOperator, value, condition.FieldValue))
           {
               tempBool = true;
               break;
           }
           else if (filter.LogicalOperator == LogicalOperator.And)
           {
               tempBool = ApplyCondition(condition.ConditionOperator, value, condition.FieldValue);
               if (!tempBool)
               {
                    break;
               }
           }
       }
 }

EDIT

It gets even simpler:

 foreach (var condition in filter.ConditionNodes.Where(a => a.IsActive))
 {
      tempBool = false;
      if (!string.IsNullOrWhiteSpace(condition.FieldName) && values.ContainsKey(condition.FieldName))
      {
           var value = values[condition.FieldName];
           tempBool == ApplyCondition(condition.ConditionOperator, value, condition.FieldValue);

           if ((filter.LogicalOperator == LogicalOperator.And && !tempBool) || (filter.LogicalOperator == LogicalOperator.Or && tempBool))
           {
                break;
           }
       }
 }

In the if you need ApplyCondition to be true and then set tempBool to true (also the result of ApplyCondition). In the else if you set tempBoolto the result of ApplyCondition. That means you can set tempBoolto the result of ApplyConditionin the first place. Now you just need to decide if you need to break.

Upvotes: 3

Polyfun
Polyfun

Reputation: 9639

Taking a more o-o approach, I think your operators need to be defined by classes that inherit from a base class. The base class would have an abstract Evaluate method that your operators implement. You can then use o-o polymorphism to evaluate your operators without worrying about the internal details. Effectively you have the beginnings of a simple interpreter.

Upvotes: 1

Related Questions