1

I have always written my if statements like this when I want the negative condition to enter me into the if block.

Example 1

if(condition == false){}

However, we just hired a new senior on the team that insists we should refactor to

Example 2

if(!condition){}

and use that moving forward.

I find example #1 to be easier to reason especially when checking a nested property.

e.g. if(person.name.middle.hasValue == false){}

Question What example is better to practice? or is there a better practice than either example?

Edit Scope please limit answers to Negative conditions only. of course if(condition == true){} is worst than if(condition){} Because the == true is redundent for the true case but not the false case.

Luke Hammer
  • 145
  • 9
  • 1
    _"if(condition == false)"_ is obviously redundant and not necessary. Your senior is right about refactoring that. I also don't see how _nesting_ would make a difference here. – πάντα ῥεῖ Feb 18 '20 at 19:47
  • 1
    @πάνταῥεῖ It's not redundant. `condition == true` is redundant, but `== false` is the same as `!` - if you leave it out, you'll get wrong behaviour. – R. Schmitz Feb 18 '20 at 19:50
  • 9
    **Readability is King,** which is why, like many things in software development, there's no *absolute* answer to this question. As a general rule, however, any reasonably proficient C# developer will see `if(!person.name.middle.hasValue)` exactly for what it is. If your find that hard to read, the better solution is to eliminate some of the dots, not tack `= false` onto the end of it. – Robert Harvey Feb 18 '20 at 19:51
  • @R.Schmitz Of course that's redundant regarding the `!` operator. – πάντα ῥεῖ Feb 18 '20 at 19:52
  • @πάνταῥεῖ I think you mean a different word? You can't leave it out without changing the logic = it's not _redundant_. – R. Schmitz Feb 18 '20 at 19:53
  • @R.Schmitz Using explicit comparisons for boolean values is just redundant as I said. You can use just the value, or the negation in conditions. And yes, ***Readability is King***. – πάντα ῥεῖ Feb 18 '20 at 19:56
  • @RobertHarvey OK i think i got you. so maybe best is bool missingMiddleName = !person.name.middle.hasValue if(missingMiddleName){} ??? – Luke Hammer Feb 18 '20 at 20:10
  • 1
    @WizardHammer, no try `!person.hasMiddleName`? - introduce new method/property in Person class to encapsulate child members. – Fabio Feb 18 '20 at 20:24
  • 3
    @πάνταῥεῖ The word you are looking for is **equivalent** (which means that `!` and `== false` are interchangeable), not _redundant_ (which would mean that `condition == false` and `condition` are exactly the same) – Flater Feb 18 '20 at 20:32
  • @gnat NO that question is different(though related). while == true is always redundant. The same is not true for == false. you can see flater and πάνταῥεῖ talking about this in the comments. – Luke Hammer Feb 18 '20 at 22:17
  • I don't find it important, however if you feel it is, here is another question that explicitly covers the matters you mentioned: [Why Use !boolean_variable Over boolean_variable == false](https://softwareengineering.stackexchange.com/q/136908/31260) (these topics were generally very thoroughly covered in prior questions) – gnat Feb 18 '20 at 22:39
  • _we just hired a new senior_ - What is the reasoning this new senior provides to change a way you write `if` statements? – Fabio Feb 19 '20 at 22:07
  • 1
    I'll just throw this in as a comment - you're all wrong wrong. It should be `if (false == condition) ..` in order to avoid accidental assignments :D – Peter M Feb 20 '20 at 14:50
  • @PeterM Ahh, the famous Yoda condition - but for bools! – R. Schmitz Feb 21 '20 at 15:23

4 Answers4

4

Personally, I prefer if (condition) or if (!condition) to if (condition == true) or if (condition == false) or worse, if (condition != true), because the latter three look like someone did not fully understand how if- (and other) conditions work.

Beginners often think that there must be some kind of comparison in an if-statement; however, this is not the case. What these kind of statements (if, where etc.) require, is a Boolean expression. I.e., an expression yielding either true or false. expr == true yields true when expr yields true, and false when expr yields false. So, the == true part doesn’t change the result of the expression. It's like writing int y = x * 1; instead of just int y = x;.

condition == false makes more sense than condition == true, but still smells like the latter.

2

This is not a question where you will get a direct answer, because both work. Most people will tell you it's a matter of taste. My personal opinion is: Don't lose sight of the end goal readability.

My Favourite in a Perfect World

I prefer to write code that's "small and easy"; simple statements that are rather short. I prefer to use ! in that code:

if(!IsReadable) SkipWord();

Reality

At work we have a legacy project which - diplomatically said - is written in a way I personally don't prefer. Using == false makes it more readable. Imagine something like

if(word.IsReadable && sentence.HasSubclause && customer.HasAlreadyChosen == false
 || word.IsReadable == false && order.IsCompleted)

It can go on longer, but I'm gonna stop here. Don't pay much notice to how you could recombine the statements - the point is it's a lot of stuff, a lot of "noise" and it would be easier to accidentally ignore a tiny ! in there.

Best would be to make the code overall more readable, but if a tool (resharper etc) can apply one of the styles throughout the codebase automatically, this is a way to prevent a whole bunch of bugs.

In Summary

In my opinion: Use ! if your code is clean enough that it still stands out.

And, probably obvious, but whichever one you end up using, never do condition == true.

And finally, mostly you'll work in a team and will have to use what the team agrees on, even if it would be an obviously worse decision.

R. Schmitz
  • 2,608
  • 1
  • 16
  • 28
1

In my opinion, if using if (!condition) is any less readable than if (condition == false) then you have other problems with your conditional statement which you should work to resolve.

The ! negation option also cuts down on visual clutter. Extra == false comparisons in the conditional increases the number of tokens you need to visually process, especially in more complex conditionals.

if (!isFeatureEnabled || (!isUserAuthorized && !isTestMode))
// --- rather than ---
if (isFeatureEnabled == false || (isUserAuthorized == false && isTestMode == false))

// ::: possible refactor :::
if (!isFeatureEnabled || !(isUserAuthorized || isTestMode))
// --- rather than ---
if (!isFeatureEnabled || ((isUserAuthorized || isTestMode) == false))

// ::: other possible refactor :::
boolean isUserAuthorizedOrTestMode = isUserAuthorized || isTestMode;
if (!isFeatureEnabled || !isUserAuthorizedOrTestMode)
// --- rather than ---
if (isFeatureEnabled == false || isUserAuthorizedOrTestMode == false)

For all these options I like those with ! negation more. Yes, this example is pretty contrived but I could definitely see conditionals like this existing in real code.

Another note: I read my conditionals from left to right with ! read as "not", which in my opinion, combined with good variable names, makes the conditionals easier to verbalize and conceptualize than == false read as "is false".

xtratic
  • 456
  • 2
  • 11
0

Another opinion - use right tool for the job - take one from both options.

For true condition if (approved) { callMe }
For false condition if (approved == false) { callMe }

The reason for using approved == false for is false condition only because it clearly stands out in comparing with true condition if (approved).

But because the way we read code is different for different people and teams, use the way your team use. If you don't like team's approach - try to convince team to change their approach, if you can not change team's approach - change a team :)

Fabio
  • 3,086
  • 1
  • 17
  • 25
  • Thank is not a thought that I had yet. if (!approved) { callMe } would be easier to mistake for if (approved) { callMe } then (approved == false) { callMe }. – Luke Hammer Feb 18 '20 at 20:25