105
votes

There is a colleague of mine who constantly writes:

if (someBool == true)

It drives me up the wall! Should I make a big deal of it or just drop it?

13
  • 12
    I prefer the explicit if (some_flag == true) but the implicit if (is_something) or if (has_something). Note the variable names. Oct 18, 2010 at 21:44
  • 69
    Remind your colleague that the type of someBool == true is also boolean, so by the same logic it should be if ((someBool == true) == true). Oct 19, 2010 at 0:00
  • 2
    @GSto: I suppose you know it, but in PHP you can do that to "cast" anything to a bool and can be actually useful.
    – zneak
    Oct 19, 2010 at 2:21
  • 2
    @zneak Or you could be clearer and use $var = (bool) some_expression. And in most cases, it won't even matter as PHP will do the necessary conversions dynamically.
    – waiwai933
    Oct 19, 2010 at 3:15
  • 4
    always check the constant first, if (true == someBool), in case you accidentally typed = instead of == [yes i'm kidding!] Oct 19, 2010 at 17:45

47 Answers 47

126
votes

It's only redundant code, not life or death. However....

If it's happening a lot, it could be a problem with how someBool is being named. A good name can go a long way towards eliminating the need for the ==true

if(IsSomeCondition)

or

if(hasCondition)

or

if(somethingExists)

for example.

7
  • This rings most true to me. Its all about clarity and the art of naming, the original syntax is not that bad but this is the right path to better code.
    – TJB
    Oct 19, 2010 at 3:41
  • 2
    Beat me to it! Without appropriate naming, the more succinct equivalency makes no sense at all.
    – Troy Hunt
    Oct 19, 2010 at 3:42
  • 4
    I have had to used someBool == true for legacy code several times for clarity. But if I write from scratch I name the variable accordingly and use if (isSomething)
    – nimcap
    Oct 19, 2010 at 6:38
  • I agree, Even though naming would solve this, assuming the name was SomeBool, it could mean anything and be of any type (an integer equal to the quantity of booleans in an array perhaps?). Booleans need some sort of existential verb, or they will always be hard to read on their own. Oct 19, 2010 at 6:45
  • I agree, it is all about readability. If variables are named well, you don't need it. If the expression reads better with it I'd go for "== true", also it is consistent if you use "== false" instead of (the ugly) "if (!())". Oct 19, 2010 at 8:07
71
votes

When I see someBool == true, I can't help but feel like the programmer hasn't internalized the idea of evaluation, which is a pretty fundamental deficiency.

However, my perspective is skewed because I spent several summers in college teaching programming to kids, who frequently wrote expressions like this because they genuinely hadn't mastered the mental exercise of evaluating an expression. Once they grokked the concept, the redundancy became obvious.

For an otherwise competent professional programmer, this is probably not the case. It's probably just a bad habit they developed in their early days of programming and never quite shook. But it would still scare me a bit if it was the first thing I saw someone do in an interview.

5
  • I wouldn't so quickly chalk this up to habit. I've heard some truly stunning things out of professional programmers' mouths. Usually followed shortly thereafter by one of the grokkers, "how did this ever work?" Oct 18, 2010 at 23:18
  • 12
    Agree wholeheartedly about evaluation. On occassion I've wondered, "Where does it stop?" if ((((x == true) == true) == true) != false) // and so on...
    – yawmark
    Oct 19, 2010 at 3:52
  • 1
    Sometimes I would write if (c == true) return true; else return false;, but 99% of the time (the 1% is there since I can never be sure that I didn't miss some) I'll immediately notice and replace the whole thing with return c. I'll expect most competent programmers should do something similar if they developed the habit early on their career. What I wouldn't expect is a wtf response.
    – Lie Ryan
    Oct 19, 2010 at 11:54
  • 1
    Oh boy, the art of thinking. I literally came across this in our codebase last week. if (!someCondition) { someCondition = false; } I've seen some (er, many) redundancies as well as some impossibilities in our code, but this one was so simple yet so aggravating to think that somebody actually wrote it. Multiple times, even. Oct 20, 2010 at 19:26
  • 1
    Just note that I have seen a few cases of if(x) x=true; which were NOT redundant, but rather the equivalent of x=!!x; (normalizing x to 0/1)
    – jkerian
    Oct 20, 2010 at 20:02
58
votes

That drives me crazy too, but I'd say mention the redundancy to them in a constructive way and then drop it even if they don't concur.

Alternatively you could try this approach:
You: Can you start using the following code convention for evaluating Booleans?

if (someBool==true)&&(true==true) {}

Them: Why would we do that? The second half of that statement is redundant, it would always evaluate to true.

You: By George, you are right. Silly me. Let's just go with a version without all the redundancy then. How about?

if (someBool) {}
8
  • 6
    Yeah, agreed. It's silly, but you gotta pick your fights, and this code is indeed doing what your colleague things it will. Mention it in a non-"What the hell is WRONG with you?!" kind of way, then drop it. Though if they start pulling crap like "if (someBool != true)" or "if (!someBool == true)" or other such convolutions, that might be a fight worth picking....
    – BlairHippo
    Oct 18, 2010 at 20:11
  • 3
    How is (someBool == false) any worse than if (someBool == true)?
    – FinnNk
    Oct 18, 2010 at 20:16
  • 5
    true=true does not compile, you cannot assign to true ;-) Oct 18, 2010 at 21:41
  • 1
    I've gotten used to if(somebool == false) or !=, but always against false and not true. I find it redundant, but since the coding standard insists that if() looks like a function call the whitespace between the parens helps me read it. On my own, a bool is a bool, but if (somePointer) doesn't fly; I prefer if (somePointer != NULL) since a pointer is not a bool. Oct 18, 2010 at 23:05
  • 5
    It's about readability, not illogic or (micro)-redundancy Oct 19, 2010 at 8:14
44
votes

I think that, if something so trivial is your biggest problem with your co-workers, you should consider yourself pretty lucky.

3
  • 5
    Well put, its good to strive for perfection but surely there are bigger fish to fry
    – TJB
    Oct 19, 2010 at 3:43
  • 3
    I think you say this about every problem that merits discussion. Oct 19, 2010 at 6:40
  • 2
    It's potentially indicative of much larger problems. A small brown stain on the ceiling in your garage may not seem like a big deal, but it could mean you have a major water leak.
    – Josh
    Oct 19, 2010 at 12:52
42
votes

You should definitely stop this bad habit. Gently...

It's easy to forget to write the double equal signs, turning the code into:

if (someBool = true)

In C# for example this will only produce a warning, not an error. So unless you treat warnings as errors the code will run, set the variable to true and always enter the condition.

21
  • 6
    This isn't a relevant argument. If you're working in a language like that, you could just move the true to the other side. The question is about whether or not to include the true at all.
    – Cam
    Oct 18, 2010 at 20:18
  • 8
    @Cam: Missing the point are you. Backwards logic I not am suggesting.
    – Guffa
    Oct 18, 2010 at 20:36
  • 15
    @Cam - He is giving a real world example of when this can be a problem. I would say this is quite relevant. Oct 18, 2010 at 20:39
  • 1
    @Guffa Cam is very right. This is not a reason to stop using == (anyType) in if blocks. Oct 19, 2010 at 8:10
  • 2
    @Ronny: No, it can't happen with any type (unless the language actually allows any type as a condition). The statement if (answer = 42) {} simply doesn't compile because the expression is not a bool.
    – Guffa
    Oct 19, 2010 at 14:08
21
votes

I agree with you, but I'm going to play devil's advocate here:


Depending on the language and the variable's name, x==true is appropriate:

Consider the following situation in a language with static typing and type coercion for integral types:

if (actionsAllowed){
    //do actions, since they are allowed
    //...
}

Someone reading this section of the code might not realize immediately that actionsAllowed is a boolean variable - it could also be an integer, representing the number of allowed actions. So by adding == true, it becomes clear that x is a boolean, and not an integer being coerced to a boolean:

if (actionsAllowed == true){
    //do actions, since they are allowed
    //...
}
10
  • 13
    readability == GoodThing
    – Muad'Dib
    Oct 18, 2010 at 20:18
  • 5
    if ((readability == GoodThing) == true) ...
    – JoelFan
    Oct 18, 2010 at 20:42
  • 1
    bool isGoodThing = readibility?true:(codingStandards?true:(internalizationOfConcept?true:false));
    – johnc
    Oct 19, 2010 at 2:02
  • 17
    So rename the variable actionsAreAllowed. Oct 19, 2010 at 3:13
  • 9
    By writing if (x) { ... you are already asserting that x is a boolean or is convertible to a boolean. What you call it is irrelevant. Oct 19, 2010 at 10:28
15
votes

What about nullable bools?

bool? MyFlag = null;

if (MyFlag == true)
    ;  

if (MyFlag)  // error, doesn't compile!
    ; 
8
  • if (MyFlag.Value)
    – johnc
    Oct 19, 2010 at 2:05
  • 1
    Not all languages have nullable bools, and many of those that do will accept your second construct.
    – zneak
    Oct 19, 2010 at 2:30
  • 1
    @johnc: "if (MyFlag.Value)" may not really do what you want it to as it can throw an exception if MyFlag is null (which you can test by checking MyFlag.HasValue). Oct 19, 2010 at 3:36
  • 4
    This is my pet peeve with nullable bools :) Oct 19, 2010 at 4:48
  • 3
    bool? isValid = null; if (isValid ?? false) ;
    – skolima
    Oct 19, 2010 at 5:47
11
votes

Typically, you don't want to make a big deal out of coding conventions unless said convention is somehow impeding the project in a significant way. I've seen many a heated argument escalate over things as small as code regions and leading underscores.

That being said, I see no issue with adding == true to a conditional statement. As a matter of fact, I am in the habit of using == false as opposed to a leading exclamation point when testing for negative conditions. I think it's more readable.

If there is an established convention, I say follow it unless there is reason to change. However, it's not really worth raising a big stink about.

2
  • 1
    Depending on your language, someValue might not necessarily be the equivalent of true even if it evaluates to true. For instance, (9 == True) evaluates to false in Python and I would imagine similar in C++. Oct 18, 2010 at 23:06
  • code regions and leading underscores make me want to punch people in the face.
    – MGOwen
    Oct 19, 2010 at 3:42
10
votes

Ack. I'm that guy. The shame, the shame. It's how I learned, and it's how I "auto-format" in my head. The only time I use Joel's prefered syntax is when the bool variable has a verb prefix like "is." I need the verb, be it "is," "can," "did," or else I need the == to provide the verb "equals." I might never break that habit, so I'll understand if you don't say 'hello' to me on the street.

1
  • 7
    It is not a bad thing. Code that reads like real text is far better, than implicit foo. Keep that habit for the sake of readability. Oct 19, 2010 at 8:18
10
votes

remind me of "boolean madness code", its like these

if(someBool == true)
   otherBool = false;
else
   otherBool = true

Instead of:

 otherBool = !someBool
1
  • That's funny, I had to maintain a system that had those littered all over. It drove me insane.
    – Tjaart
    Oct 20, 2010 at 9:15
8
votes

Personally, I strongly dislike the way to say "not" in C based languages. That little exclamation mark is too easy to overlook.

Hence I write it out in full:

if (someCondition == false) {

After reading that for a while, I want symmetry too with

if (someCondition == true) {

So consider it an artifact of C using ! instead of not.

2
  • 3
    C++ actually has the not operator, and so does C (once you include the standard header <iso646.h>). If you don’t want to (or can’t) use this header (or if you’re stuck with Java or C#), I suggest putting a space after the exclamation mark: if (! condition). This makes it somewhat more conspicuous. Oct 21, 2010 at 7:28
  • 1
    I do Java. not is not an option.
    – user1249
    Oct 21, 2010 at 8:13
6
votes

It depends on the language, but it's usually a bad idea...

In C, never do this. It's too easy to find a situation where the value you are testing is not false (non-zero), but also not equal to the single value defined as "true".

In Ruby, do this only if you are absolutely certain that you want to fail on everything except Boolean true.

In C++ and other static languages with a bool type, it's redundant, and can lead to programming errors when you mis-type = instead of ==, or promotion errors as mentioned in the comments.

3
  • Actually, in languages where the assignment operator returns a value ... like C++ ... if (b == true) { is not just redundant. It is also a bit risky, because you might accidentally assigned true to b.
    – Stephen C
    Oct 19, 2010 at 8:34
  • 4
    It's not just redundant in C++, it's dangerous. If you write if (b == true), and b isn't a bool, the type conversions go the wrong way. A bool is an integral type that is normally promoted to an appropriate integral type with value 1. If you write, say, int b(2); if (b == true) then true becomes an int with value 1 for purposes of the comparison, rather than b being coerced into type bool, which would give the right result. Oct 19, 2010 at 18:02
  • @DavidThornley, turn on warnings in your compiler. Treating warnings as errors is better defensive programming technique rather than avoiding ==.
    – Abyx
    Dec 8, 2011 at 16:12
4
votes

I prefer

if (bVal)

or

if (!bVal)

too, but I'm afraid that bringing it up would piss people off, so my advice is to forget it. Sorry!

2
  • 4
    For the love of all that is holy, as long as it's not if (!bNotVal) or if (bNotVal) even in the first place. Ugh negatives in names makes everything harder to read. Oct 18, 2010 at 23:48
  • 2
    I knew a dev who would bool isNotConnected; if (isNotConnected==true) ... yuck
    – johnc
    Oct 19, 2010 at 2:04
4
votes

you should tell him that he is doing it wrong.

its

if (true == someBool) {

}

if he ever forget one = he is in big trouble in his writing style.

4
votes

You think that's bad? How about:

if(someCondition) {
  return true;
} else {
  return false;
}
2
  • 2
    Boy how many times I have seen this. This makes a good case for a tool like ReSharper.
    – Job
    Nov 27, 2010 at 19:12
  • Yes - if you hire clods, then buy ReSharper. Nov 22, 2011 at 5:19
3
votes

How about

if (x == "true")

WHY IS THAT A STRING?!

3
  • I'm working on some legacy php code right and occasionally I find something like if(preg_match('/title/', implode($_POST))){. PHP, enough said, I need to find a better job.
    – Benbob
    Oct 19, 2010 at 5:23
  • 4
    yea, I also see if (x == "1"), but that's usually do to poor db design.
    – atfergs
    Oct 19, 2010 at 12:53
  • I've used similar constructs when x came from user input (configuration file or web service, for example). However, I usually allow other truish values, so it ends up more like if x.lower().strip() in ["true", "yes", "on", "1"]
    – eswald
    Oct 19, 2010 at 17:17
3
votes

I write code like that!

Here is why:

"if bla == true" reads like a sentence, where as "if bla" does not in many cases. It just sounds wrong, when READING actual code.

Also the compiler warns about assignments in if blocks, so there is really no danger in using == true. (confusing it with =)

Also do guys that don't write "== true", use that "!()" for "== false"? I find it really ugly. And if you use "== false", it is only very consistent to also use "== true", instead of having two distinct ways of verifying truth.

2
  • 3
    You say "if bla" does not read like a sentence... that means your variable is not named properly... what's wrong with this: if (userHasRights) doSomething()
    – JoelFan
    Oct 19, 2010 at 13:13
  • "in many cases". Yes, you are right. Renaming is fine, too. With "if (QWidget::acceptDrops() == true)" you don't have the possibility to rename. perhaps it would be good to name it doesAcceptDrops() ... It's too much of a hassle (and it's impossible to be consistent using that omission-rule in any API), so being consistent with other "== whatever"-ifs, I strongly suggest not omitting it, so it doesn't "look" different, thus looks consistent. Oct 19, 2010 at 13:34
2
votes

Generally will omit the '== true', but it's hardly worth even a minute discussing it unless you have it included in your team's coding standards.

2
  • 3
    Also, if it is in the team's coding standards, you really need to re-evaluate the standards to get rid of trivia like this.
    – JohnFx
    Oct 18, 2010 at 20:11
  • Agreed! Just in case though...
    – FinnNk
    Oct 18, 2010 at 20:15
2
votes

While I agree as a mainly C# developer, I can't say this is always the case. For instance, in Javascript, the === will perform type coalescence. So assuming var x = 3 then:

if(x) --> true

while

if (x === true) --> false

I guess that's different than == since even in JS I wouldn't use if(x == true) but just something to think about.

This sort of touches on another point though which has come up in my office:

bool b = false;

In C#, bool b; would be enough and would initalize b to false. However, it is more explicit to write the above line and anyway should be ripped out by the compiler during optimization.

So I guess my point is it's not always so obvious what is and isn't good practice and a lot of it boils down to preference as well as language features/quirks.

2
  • bool b; only initializes to false when b is a field. You must initialize local variables explicitly. Oct 19, 2010 at 5:55
  • +1 agreed. Its the same issue with other (scripting) languages as well. You must be explicit if you want to test for boolean true or just "true". For instance ie any non-empty string is considered == true but not === true in some languages. Oct 29, 2010 at 7:22
2
votes

Actually, if it's nullable, testing for x == true would be necessary. :)

2
votes

Ah yes, but what if the variable is nullable? (bool?)

Some languages (C#) will require and cast or comparison with 'true'.

bool? isAccepted = true;

if((bool)isAccepted)
{...}

if(isAccepted == true)
{...}
2
votes

Remember you are working as part of a team, so you need to work these things out together. "Plays nice with others" is still an important personality trait even after elementary school :)

2
votes

The young know the rules, but the old know the exceptions ;)

In latest C#, if you are dealing with a null-able bool, then you have to:

bool? x = null;
bool? y = true;
bool? z = false;
if (x == true || y == true || z == true) {
    // That was the only way that is reasonably readable that I know of
    // to accomplish this expression.
}

If tristate is not a problem, then there usually should not be a reason to compare something to true/True. However, in Python and several other languages such as C/C++ you can perform an if on a non-bool expression. These languages have unique rules for interpreting integers, pointers, lists, etc. as either true or false. Sometime you do not want that. For example, in this Python snippet:

x = True
y = 'abcdef'
z1 = x and y
z2 = (x == True) and (y == True)

Here z should be True, but z2 should be False. Now, a Clojure language approaches this in yet another way - there and function does not necessarily evaluate to a bool, but the if can handle that.

Regardless of the language, any time you find yourself comparing something to True or False, it is probably worth commenting.

1
  • The first problem stems from a false premise IMO by using awful variable names. Suppose x,y,z were named notExceptButHasX, exceptNotButIsntY, doNotUnlessIsExceptZ? How does that make for readability in your problem? If x,y,z were named something like "isEnabled", "isEffective", "hasProperty", then your statement becomes isEnabled || isEffective || hasProperty which is far more readable than comparing to true or false.
    – Thomas
    Mar 20, 2011 at 2:46
1
vote

Such coding would have rubbed me the wrong way before too. Although your example identifier is named "someBool", using that coding style inadvertently on a variable which wasn't guaranteed to be a boolean could have unintended behavior. If the value of "someBool" isn't exactly "true" or "false", the result will be false.

I encountered a very subtle bug this past year caused by such a coding style which was difficult to identify because one's eyes gloss over such constructs. You'd think, "How could it be wrong?" The same holds true for such well-understood expressions as "(var)" or "(!var)", that you read or code them without verifying their behavior.

So I've introduced a couple of coding standards to reduce the existence of such bugs in the codebase and the likelihood that such subtle bugs will accidentally creep in sometime in the future.

  1. All expressions must be parenthesized per their intent.
  2. All boolean tests must be expressed in the negative, like "suchBool != false".

By cleaning up code not conforming to the new style, I've identified and corrected a few more instances of such subtle bugs.

2
  • 1
    Having some someBool able to be something other than TRUE/FALSE is bad coding practice. Oct 19, 2010 at 0:20
  • @AttackingHobo, that's one of the pitfalls of not having boolean types in a language and/or not using a class to provide the exact behavior wanted. Oct 19, 2010 at 11:36
1
vote

Had to use this all the time in ActionScript 2 (admittedly now a dead language), because:

var something:Boolean = org.black.box.unknown.ThisMightNotExistYet();

// this is slightly ambiguous
if(something)
{
}

// because if you allow undefined you might actually mean
if(false != something)
{
}

// which can mean something different than
if(true == something)
{
}

// and comparing against what you actually MEAN is less script than
if(undefined != value && value)
{
}

So it was almost always best to be specific.

1
vote

I agree. It's a redundant construction, specially in strong typed languages.

To add another misuse of booleans, I have found this kind of construction a bunch of times in Javascript, (specially at some spaghetti-like monster functions, as in 100+ lines):

//create the variable, not initializing it
var flag;
//...
//assing a value to the var, by the example 
flag=$("mycheckbox").selected;
//...

//and at the moment of use it:
if(flag!=true) {
   //code to execute when the checkbox is unchecked
}

It seems, that due to the lack of an strict type definition in this language, some programmers prefer not have to be messing around with the false|undefined values.

1
vote

I have a colleague who will have some code like this:

if(something == true)

And then, for some sort of test/debugging, he will wish to not call this block so he'll change it to:

if(something == true && false)

And then occasionally he'll change it to:

if(false)

The worst thing is, this type of debugging has rubbed off on me on occasion and is really bad for other developers to read!

0
votes

I'd say consistency is king in a codebase. So you should use the style, that is mostly used in your organization. Try to make your preferred style part of official company coding guidelines. If it's already in the guidelines, just follow it.

(That being said, it would also really annoy me - but probably not enough to make a big deal out of it).

0
votes

I prefer not placing the extra == true, but sometimes I accidentally include it and don't notice it. The person may not have noticed and accidentally placed them. I reread my code and sometimes notice that I placed the extra == true so I remove that == true. I sometimes don't notice it and would happily welcome someone telling me I placed it redundantly.

0
votes

how about:

int j = 1;
for (int i=1; i>=j; i++) ...

Yep, I've seen it.

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