Merge pull request #75 from bijington/issue-73-boolean-not-short-circ…

…uiting Boolean AND and OR expressions now correctly short circuit if the lhs…
bijington · Aug 24, 2020 · 090043c · 090043c
2 parents 7c89bb7 + 71863f1
commit 090043c
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 9 deletions.
diff --git a/Source/CSharp/Expressive/Expressive.Tests/Expressions/Binary/Logical/AndExpressionTests.cs b/Source/CSharp/Expressive/Expressive.Tests/Expressions/Binary/Logical/AndExpressionTests.cs
@@ -52,5 +52,20 @@ public void TestRightTrueEvaluate()
 
             Assert.AreEqual(false, expression.Evaluate(null));
         }
+
+        [TestMethod]
+        public void TestShortCircuit()
+        {
+            var rightHandMock = new Mock<IExpression>();
+
+            var expression = new AndExpression(
+                Mock.Of<IExpression>(e => e.Evaluate(It.IsAny<IDictionary<string, object>>()) == (object)false),
+                rightHandMock.Object,
+                new Context(ExpressiveOptions.None));
+
+            expression.Evaluate(null);
+
+            rightHandMock.Verify(e => e.Evaluate(It.IsAny<IDictionary<string, object>>()), Times.Never);
+        }
     }
 }
diff --git a/Source/CSharp/Expressive/Expressive.Tests/Expressions/Binary/Logical/OrExpressionTests.cs b/Source/CSharp/Expressive/Expressive.Tests/Expressions/Binary/Logical/OrExpressionTests.cs
@@ -52,5 +52,20 @@ public void TestRightTrueEvaluate()
 
             Assert.AreEqual(true, expression.Evaluate(null));
         }
+
+        [TestMethod]
+        public void TestShortCircuit()
+        {
+            var rightHandMock = new Mock<IExpression>();
+
+            var expression = new OrExpression(
+                Mock.Of<IExpression>(e => e.Evaluate(It.IsAny<IDictionary<string, object>>()) == (object)true),
+                rightHandMock.Object,
+                new Context(ExpressiveOptions.None));
+
+            expression.Evaluate(null);
+
+            rightHandMock.Verify(e => e.Evaluate(It.IsAny<IDictionary<string, object>>()), Times.Never);
+        }
     }
 }
diff --git a/Source/CSharp/Expressive/Expressive/Expressions/Binary/BinaryExpressionBase.cs b/Source/CSharp/Expressive/Expressive/Expressions/Binary/BinaryExpressionBase.cs
@@ -49,7 +49,7 @@ public object Evaluate(IDictionary<string, object> variables)
 
         #endregion
 
-        private static object CheckAndEvaluateSubExpression(object result, IDictionary<string, object> variables)
+        protected static object CheckAndEvaluateSubExpression(object result, IDictionary<string, object> variables)
         {
             if (result is Expression lhsExpression)
             {

diff --git a/Source/CSharp/Expressive/Expressive/Expressions/Binary/Logical/AndExpression.cs b/Source/CSharp/Expressive/Expressive/Expressions/Binary/Logical/AndExpression.cs
@@ -15,10 +15,9 @@ public AndExpression(IExpression lhs, IExpression rhs, Context context) : base(l
 
         #region BinaryExpressionBase Members
 
-        protected override object EvaluateImpl(object lhsResult, IExpression rightHandSide, IDictionary<string, object> variables)
-        {
-            return this.Evaluate(lhsResult, rightHandSide, variables, (l, r) => Convert.ToBoolean(l) && Convert.ToBoolean(r));
-        }
+        protected override object EvaluateImpl(object lhsResult, IExpression rightHandSide, IDictionary<string, object> variables) => 
+            Convert.ToBoolean(CheckAndEvaluateSubExpression(lhsResult, variables)) &&
+            Convert.ToBoolean(CheckAndEvaluateSubExpression(rightHandSide.Evaluate(variables), variables));
 
         #endregion
     }

diff --git a/Source/CSharp/Expressive/Expressive/Expressions/Binary/Logical/OrExpression.cs b/Source/CSharp/Expressive/Expressive/Expressions/Binary/Logical/OrExpression.cs
@@ -15,10 +15,9 @@ public OrExpression(IExpression lhs, IExpression rhs, Context context) : base(lh
 
         #region BinaryExpressionBase Members
 
-        protected override object EvaluateImpl(object lhsResult, IExpression rightHandSide, IDictionary<string, object> variables)
-        {
-            return this.Evaluate(lhsResult, rightHandSide, variables, (l, r) => Convert.ToBoolean(l) || Convert.ToBoolean(r));
-        }
+        protected override object EvaluateImpl(object lhsResult, IExpression rightHandSide, IDictionary<string, object> variables) =>
+            Convert.ToBoolean(CheckAndEvaluateSubExpression(lhsResult, variables)) ||
+            Convert.ToBoolean(CheckAndEvaluateSubExpression(rightHandSide.Evaluate(variables), variables));
 
         #endregion
     }