Implement SA1131 (UseReadableConditions)

[sharwell]

Category: Readability
Rule: Use readable conditions (alternatively: Do not use Yoda conditions)
Description: The equality and inequality operators == and != are generally symmetric. In combination with #862, there is no longer a need to write expressions with a literal as the first operand. This rule is described as follows:

The first operand of an equality operator should not be a literal (or null), unless the second operand is also a literal (or null).

[vweijsters]

👍

Yoda conditions had their uses in C / C++, but given the strict rules about valid conditions in C#, it no longer makes sense.

[Przemyslaw-W]

+1
Similarly, if we stick to "use readable conditions", this could also cover
if (booleanExpression) or if (!booleanCondition)
instead of
if (booleanCondition == true) or if (booleanCondition == false)

2015-05-18 21:49 GMT+02:00 Vincent Weijsters notifications@github.com:

[image: 👍]

Yoda conditions had their uses in C / C++, but given the strict rules
about valid conditions in C#, it no longer makes sense.

—
Reply to this email directly or view it on GitHub
#863 (comment)
.

[sharwell]

@Przemyslaw-W avoiding == true and == false should be a different rule, and needs further discussion due to potentially problematic interaction with #862.

[pdelvo]

grabbed.

[alxru]

I'd like to 👍 the proposal from @Przemyslaw-W to generate a warning when an expression of boolean type is compared to false or true. For example, the warning should be generated in the following cases:

if (completedSuccessfully == true)
if (control.IsVisible == false)
if (dataStorage.GetValue("name") == true)
if ((bool)data[i] == true)

[pdelvo]

Im currently working on this. Do we want to report on all occurences of '==' that has a constant value on its left and a non constant value on its right or only built in ones? You could write a non commutative == operator.

[sharwell]

@pdelvo We should assume that operator == is commutative. Are there any cases where changing the order of operands can change which operator gets resolved (I'm thinking of implicit type conversions here)?

[nriding]

I don't like this rule. I've been coding C/C++/C# for a long time. I've always written my comparisons <constant> == <value>, it's how all my code is formatted. I find it easier to read. The most important term come first. After updating StyleCop I now have to disable this rule in all my projects.

[GregReddick]

Just make sure complaining about the comparison to true or false is only for bool and not the bool? data type, where it is valid.