0

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.

  • 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 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 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 at 19:51
  • 1
    @WizardHammer, no try !person.hasMiddleName? - introduce new method/property in Person class to encapsulate child members. – Fabio Feb 18 at 20:24
  • 2
    @πάνταῥεῖ 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 at 20:32
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.

|improve this answer|||||
  • 1
    if (!IsReadable) - If not is readable ;) – Fabio Feb 19 at 2:47
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.

|improve this answer|||||
  • 1
    "Looks like someone didn't fully understand" is not the reason you avoid it. – Robert Harvey Feb 18 at 19:54
  • 1
    It's my reason. But of course, both variants are correct. – Olivier Jacot-Descombes Feb 18 at 19:58
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 :)

|improve this answer|||||
  • 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 }. – WizardHammer Feb 18 at 20:25
0

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".

|improve this answer|||||

Not the answer you're looking for? Browse other questions tagged or ask your own question.