Why is it bad to use redundancy with logical operators? [duplicate]

Question

I'm moving over to work on a library that a fellow developer has been writing. It's full of == true and == false, which I find crazy frustrating to read.

I've tried asking him to quit doing it, but he just says that it makes it more straightforward to understand what is going on. Is there a good reason why this isn't a great practice? Or is it purely a stylistic thing?

Answer 1

The bigger the code the more time it takes to read and the more chance you have to misunderstand something.

== true is 7 characters that contribute nothing to the code and thus is pure cost, no benefit.

I wouldn't be so harsh on == false, though. You can make an argument between negation and a comparison to false, while you can argue negation is superior it's not absolutely trumped like in the == true case.

Answer 2

Depending on the language, the semantics are not so trivial. In Python:

class X:
    def __cmp__(self, other):
        return 1
    def __nonzero__(self):
        return False

if X():
    print "'X()' evaluated to: true"
else:
    print "'X()' evaluated to: false"

if X() == False:
    print "'X() == False' evaluated to: true"
else:
    print "'X() == False' evaluated to: false"

if X() == 0:
    print "'X() == 0' evaluated to: true"
else:
    print "'X() == 0' evaluated to: false"

if X() is False:
    print "'X() is False' evaluated to: true"
else:
    print "'X() is False' evaluated to: false"

if X() is None:
    print "'X() is None' evaluated to: true"
else:
    print "'X() is None' evaluated to: false"

prints

'X()' evaluated to: false
'X() == False' evaluated to: false
'X() == 0' evaluated to: false
'X() is False' evaluated to: false
'X() is None' evaluated to: false

Certainly one can make a case, even in python, for ignoring the operator. The problem is that it may be misleading for who wrote the code ("python must be just like [whatever-language], so I'll leave the equality operator implicit") and for who reads it ("was he really intending to eq compare implicitly or should it be an explicit is?").

It is very time consuming to track down bugs caused by implicit boolean checks that should be explicit (and correct).

Answer 3

I too find this frustrating, up there with

if (condition)
    return true;
else
    return false;

It indicates to me a lack of understanding of what booleans do. Any additional complexity in code is a potential source of bugs, although this one is trivial enough I could put it off to stylistic differences. You can get used to anything.

Answer 4

I agree with your point, and I too find it frustrating to read; however, when you have a lone negated if condition like so:

if(!condition)
    doThis();

I've been told by a few senior devs that it can be easy to miss the negation operator when reviewing code and they prefer to see:

if(condition == false)
    doThis();

I agree with you that it is too verbose; however, it seems that in some instances like the one I described, some people would rather have that.

Answer 5

It isn't, in general. For compiled languages the expression will be likely reduced to the same machine code, and in interpreted languages the explicit test will help reduce the chance of unwanted code insertion.

Consider the following JavaScript function, to, say, tally votes on a hypothetical ballot measure.

function recordVote(YesOrNo) {
  if(YesOrNo == true) {
    voteYes();
  } elseif (YesOrNo == false) {
    voteNo();
  }
}

If you call recordVote(true) or recordVote(false), the interior method fires as expected. And, if you improperly call recordVote(42), recordVote(null), or recordVote("false"), you get no action -- which is desired for a voting app. (especially since the standard test for truthyness, of if(YesOrNo), would cause that last one to record a vote incorrectly.

---

Of course, note that the small benefits you get from the above are inferior to consistency in your code. If the library's already full of them, don't remove them -- but don't add them if it isn't. (Unless, of course, you're refactoring everything anyway and have some arbitrary style rule to force your source code to be as small as possible...)