Reputation: 506
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
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
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 tempBool
to the result of ApplyCondition
. That means you can set tempBool
to the result of ApplyCondition
in the first place. Now you just need to decide if you need to break.
Upvotes: 3
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