Fix logical operators task.

smowton · smowton · commit fe0398dd3af5 · 2021-03-22T10:51:00.000Z
This was asking for a goal that conflicted with moreWaysToReturn.go -- logicalOperators.go considered paths that may circumvent a return statement as acceptable, while moreWaysToReturn.go wanted these maybe cases flagged as problems. Here we take moreWaysToReturn.go as correct and amend the good/bad labels in logicalOperators.go accordingly, also adding a few more test cases.
diff --git a/logicalOperators.go b/logicalOperators.go
@@ -50,76 +50,134 @@ func someOtherCondition() bool {
 	return true
 }
 
-func logicalAndThenBranchGood() {
+func logicalAndThenBranchSometimesBad() {
 
 	if errorSource() != ErrNone && someOtherCondition() {
-		// Good: in the then-block, where we're certain there has been an error,
-		// we return early. In the other branch an error is possible but not certain. 
+		// Bad: there is a route from a positive error test around the 'return' statement.
 		return
 	}
 	doSomething()
 
 }
 
-func logicalAndThenBranchBad() {
+func logicalAndThenBranchAlwaysBad() {
 
 	if errorSource() != ErrNone && someOtherCondition() {
-		// Bad: in the then-block, where we're certain there has been an error,
-		// we do not return early.
+		// Bad: there is no return statement at all.
 		insteadOfReturn()
 	}
 	doSomething()
 
 }
 
-func logicalAndElseBranchUncertain() {
+func logicalAndElseBranchAlwaysBad2() {
 
 	if errorSource() == ErrNone && someOtherCondition() {
 		doSomething()
 	} else {
-		// Uncertain: an error is not a precondition for entering either branch.
-		// We should be conservative and NOT flag this function as a problem.
+		// Bad: On error we may enter either branch, but neither one returns.
 		insteadOfReturn()
 	}
 	doSomething()
 
 }
 
-func logicalOrElseBranchGood() {
+func logicalAndThenBranchGood() {
+
+	if someOtherCondition() && errorSource() != ErrNone {
+		// Good: whenever an error is indicated we return.
+		return
+	}
+	doSomething()
+
+}
+
+func logicalAndElseBranchGood() {
+
+	if someOtherCondition() && errorSource() == ErrNone {
+		// Good: whenever an error is indicated we return.
+		doSomething()
+	} else {
+		return
+	}
+
+}
+
+func logicalAndElseBranchGood2() {
+
+	if errorSource() == ErrNone && someOtherCondition() {
+		// Good: whenever an error is indicated we return.
+		doSomething()
+	} else {
+		return
+	}
+	doSomething()
+
+}
+
+func logicalOrElseBranchSometimesBad() {
 
 	if errorSource() == ErrNone || someOtherCondition() {
 		doSomething()
 	} else {
-		// Good: in the else-block, where we're certain there has been an error,
-		// we return early. In the other branch an error is possible but not certain. 
+		// Bad: there is a route from a failing error test that bypasses the return statement.
 		return
 	}
 	doSomething()
 
 }
 
-func logicalOrElseBranchBad() {
+func logicalOrElseBranchAlwaysBad() {
 
 	if errorSource() == ErrNone || someOtherCondition() {
 		doSomething()
 	} else {
-		// Bad: in the else-block, where we're certain there has been an error,
-		// we do not return early.
+		// Bad: regardless of error status, we do not return.
 		insteadOfReturn()
 	}
 	doSomething()
 
 }
 
-func logicalOrThenBranchUncertain() {
+func logicalOrThenBranchAlwaysBad() {
 
 	if errorSource() != ErrNone || someOtherCondition() {
-		// Uncertain: an error is not a precondition for entering either branch.
-		// We should be conservative and NOT flag this function as a problem.
+		// Bad: regardless of error status, we do not return.
 		insteadOfReturn()
 	} else {
 		doSomething()
 	}
 	doSomething()
 
 }
+
+func logicalOrThenBranchGood() {
+
+	if someOtherCondition() || errorSource() != ErrNone {
+		// Good: whenever an error is indicated we return.
+		return
+	}
+	doSomething()
+
+}
+
+func logicalOrElseBranchGood() {
+
+	if someOtherCondition() || errorSource() == ErrNone {
+		// Good: whenever an error is indicated we return.
+		doSomething()
+	} else {
+		return
+	}
+
+}
+
+func logicalOrThenBranchGood2() {
+
+	if errorSource() != ErrNone || someOtherCondition() {
+		// Good: whenever an error is indicated we return.
+		return
+	}
+	doSomething()
+
+}
