Tuesday 14 June 2011

When true != true

If we define a boolean variable, say:
bool xyz;

then when we have to do some kind of a comparison we may do this as follows:
if(xyz == true)

When I used to be a new programmer, I used to always do this as follows:
if(true == xyz)

I think the way I did it is more robust and less error prone because you can by mistake do as follows:
if(xyz = true) //Single = instead of ==

The problem with my approach is that the code readability is affected and most people hate to see code like this. Instead, I started doing what normal people would do but making sure that I always use '==' and not '='.

Now some of you may be thinking why not:
if(xyz)

True, in case of Boolean this is a simplest way to do it and I had personally no problem with it until recently. While reviewing some code, I noticed that an engineer had changed a piece of code from:
if(xyz == true) to if(xyz)

Initially I thought that it may be that he did not like the '==' as its redundant. So when I checked his other code, I realised that he may have done this for a reason. Executing his code in the debug mode worked fine. So I changed:
if(xyz) back to if(xyz == true) and lo and behold, the test started to fail.

Putting a breakpoint I can see that while on the statement:
if(xyz == true)
the debugger shows xyz is 'true' but the comparison fails.

As usual digging into the problem further, I found that in one of the constructors, xyz was not initialised and the default garbage value appeared to show it as 'true'.

So its again got me into thinking that it may be better to write if(xyz == true) instead of if(xyz) as it may help in catching uninitialised variables. The best thing of course would be to make sure all variables in the class, whether public, protected or private are initialised.

Opinions welcome!

6 comments:

  1. I agree with the conclusion and I think "if(xyz)" is good.

    The C definition of 'if (expression) statement' states clearly that the statement is executed if the result of the expression is unequal to 0. The definition makes 'if' flexible and useful and I like it very much.

    I work for C projects which define Boolean as enum: typedef enum { FALSE, TRUE } BOOL; And our coding convention suggests to write selection statement in the way you mentioned, "if(TRUE==xyz)". This is misleading because people start to forget the original 'if'. They think the parenthesized must be comparing something to FALSE or TRUE and write stupid code like this "if((a==b)==TRUE)".

    ReplyDelete
  2. "So its again got me into thinking that it may be better to write if(xyz == true) instead of if(xyz) as it may help in catching uninitialised variables."

    That's an interesting idea, but it's not a good enough reason to add the "== true".

    C++ finally established a true boolean type, instead of the myriad ways that C programmers made booleans. As case from code I worked on once, somebody at some point created "typedef enum { True, False } Boolean" (which essentially flips the meaning of True and False). I think they did it for return values, but it wreaked havoc on any sort of conditional statement. "if(f()==True)" worked fine, but "if(f()&&g() == True)" didn't ... you had to write "if(f()==True && g()==True). Oh, and of course the traditional TRUE and FALSE macros existed and were used, just to confuse things more. "if((f()==True && g()==True)==TRUE)"

    Ever since then, I made it a point to never say "==true" or "==false" in my code, even in C++ where the problem (mostly) goes away. It's redundant, ugly, and hinders understanding. Complex conditional expressions almost always get much simpler and easier to understand when you get rid of the redundant check.

    Sure, it might help prevent those rare instances of an uninitialized boolean variable (that the compiler doesn't catch ... you do compile with all warnings turned on, right?), but it's hardly worth it.

    This brings me to a pet peeve of mine: coding standards that claim to prevent problems, but only make things worse in some way. "==TRUE" is one example. Another, much more common culprit is "if(5==i)". The claim is that it prevents accidental assignment, which is simply not true. It prevents accidental accidental assignment when testing against a constant (or some other non-modifyable value), but does not guard against the general case of "if(a=b)". Flipping the condition can only help if b something is not modifyable.

    I've seen programmers who blindly follow the advice of flipping the order of their comparisons, without training their eyes to recognize when they're using the wrong operator. That's the real solution, in the end. Well, that, and making sure you compile with all warnings turned on.

    ReplyDelete
  3. Well... since you wrote "Opinions welcome!" my opinion is:

    Anyone who can't understand the meaning of if(xyz == true) or if(xyz) and the difference between assignment (=) vs. comparison (==) may be anything EXCEPT a C++ programmer.

    What about if in the name "Advanced C++ you remove the word Advanced ?

    ReplyDelete
  4. Surprisingly, I have spent quite a lot of time debugging code failures that were eventually traced to problems like this one, written by experienced (5+ years of C++ programming experience) programmers (maybe like yourself)

    ReplyDelete
  5. >> Anyone who can't understand the meaning of ... may be anything EXCEPT a C++ programmer.
    I know you are not the anything. However:
    1) if(xyz == true) and if(xyz) are not the same, not always work as expected and often produce different result.
    2) if(true == xyz) costruct is often a requirement of a coding standard. So, better learn to read and understand.
    3) I personally prefer "if(false != xyz)". This is better portability.

    ReplyDelete