Boolean literals should not be redundant C# csharpsquid : S1125

My suggestion to remove an example#1 style as not a Noncompliant Code. For csharpsquid : S1125
With title Boolean literals should not be redundant

Example #1 (flagged as a Code Smell)
if (booleanMethod() == false) { /* ... */ }

Let’s compare this to the compliant example of this code.

Example #2 (Suggested Fix)
if (!booleanMethod()) { /* ... */ }

My paradigm for challenging this is summed up in this line from the book Clean Code pg.8 which states

“Clean code reads like well-written prose”

.
now let’s compare examples when reading that code as plain English prose

#1 if method result is false.
#2 if not method result.

I assert that “if method result is false.” is much more natural than “if not method result”.

“if not method result” requires a bit of mental gymnastics it reason about. First, I want to understand what the method result is. Second, I need to change the value of the method result into its opposite. Converting True to False and False to true.

furthermore a simple ! can easily read over. eg !earn() can read as learn() but earn() == false is unlikely to be confused the same way.

Therefore

Example 1 is more readable then Example #2 in C#.

Action request
As Sonar Cloud user I would like the code matching the example of if (booleanMethod() == false) { /* ... */ } Not to be alearted on.

thank you for time first post hear still learning the rules i welcome feedback.

Hello Luke,

I would disagree on this one. Take a look at these more basic examples:

Example #3

if(booleanMethod()) {...}

Example #4

if(booleanMethod() == true) {...}

This translates into English prose

#3 if method result.
#4 if method result is true.

Looking at this, the #4 is way more natural than #3. But from source code point of view, #4 is a bad practice and clear violation of rule S1125.

If we update methodResult() to something else

Example #5

if(!Valid) {...}

Example #6

if(Valid == false) {...}

we get another set of proses:

#5 if not valid
#6 if valid is false

In this case, #5 is better than #6 in English, but not in code.

Those true/false literal are unnecessary, just as rule suggests.

You can create your own quality profile in SonarCloud and exclude this rule if you feel it too noisy for your code base.

Pavel Mikula

First of all, let me say that I completely agree with #3 and #4.
I cannot think of an example where adding == true adds to the readability

Furthermore I’m not advocating that this rule gets destroyed I’m, just stating let’s not catch (Method() == false)

Here is my actual example that causes the code error for me

Example #7 suggested by sonarcloud
if (!log.Time.HasValue)

Example #8 not complient code which i assert is better
if (log.Time.HasValue == false)

we get another set of proses:

#7 if not log Time has a value
#8 if log Time has a value is false.

so #5 #6 is fine because when one reads not is right before valid the meaning is clear.

however in #7 #8 that does not remain true.
in #7 converted to prose, the not comes to early thus appearing to modify log or the log time instead of prase has a value.

I assert that if #7

if not log Time has a value

is more difficult to reason about then. #8

if log Time has a value is false

and as such we should consider not telling people the suggested solution is better in these cases.