Is checking for true explicity bad by design?

Question

Is it considered bad to explicitly check for the boolean true. Would it be better to do a simple if(success) ?

I've seen various jokes made about how if (someBoolean === true) is horrible code in a strongly typed language but is it also considered bad in weakly typed languages?

This would apply for any weakly typed language that does type coercion on an if statement.

A specific example would be :

var onSuccess = function (JSONfromServer) {
    // explicitly check for the boolean value `true`
    if (JSONfromServer === true) {
         // do some things
    }
}

// pass it to an ajax as a callback
doSomeAjax(onSuccess);

[Edit]

In this particular case the success variable is any valid JSON returned from a server. So it could be anything. if its the boolean true then a success happened. If it's some error handling object then it will be handled. If it's something else then it will probably be handled quietly.

The question was is getting the server to return true as JSON and checking for a good way to handle the case where the action succeeded.

I wanted to avoid being specific to JavaScript & AJAX though.

Solution 1

I'm in two minds about this myself.

In one respect, the code sample you've posted is good, because of the way Javascript handles type coercion. A simple if (success) would enter the if block so long as success was truthy - e.g. a non-empty string would do the case. The triple-equals guarantees that success is indeed the boolean value true, which is a stronger guarantee than you'd get with the shorter version (and probably one that you want).

However, if you need this - i.e. you don't know whether success will be a boolean, or a string, or an integer - I'd say that's a code smell in itself. Regardless of how you perform the comparison, I'd always compare with a variable that's going to unavoidably be a boolean; at which point, it doesn't matter which form of comparison is used as they'd be equivalent. In fact, I'd even introduce a "redundant" variable like so:

var successCount = items.size(); // or some other way to get an integer
var success = successCount > 0;
if (success) {
   ...
}

So, erm, yeah. I don't think anyone could really complain about the (explicit) use of === in the comparison because of its functional difference. But by the same token, if you're clearly using boolean success flags then I don't think someone should complain about the short style either.

(Regarding your first sentence though, I don't think it's bad to explicitly check for the boolean value true in a dynamically typed language, if that value is actually what you want. It's only superfluous when static typing already constrains the variable to be a boolean.)

Solution 2

With Javascript its worth knowing that beyond boolean true and false, values can be truthy and falsy.

Consider:

if (false)     // evaluates to false.
if (0)         // evaluates to false, 0 is falsy.
if ("")        // evaluates to false, empty strings are falsy.
if (null)      // evaluates to false, null values are falsy.
if (undefined) // evaluates to false, undefined values are falsy.
if (NaN)       // evaluates to false, NaN is falsy.

All other values for objects are truthy.

If truthy and falsy values are leading to errors in your logic, you should consider explicitly using === and !== operators to ensure objects are compared by type and value.

Solution 3

Generally, you expect boolean variable names like:

success, enabled, pass

to have a true value. So

if(success) //or

if(enabled) //or

if(pass) //or

if(enabled) //or

is understandably and logically readable. But if you have variables like:

result, status, port1.bit7

it is better to write:

if(result == true) //or

if(status == false) //or

if(port1.bit7 == true)

because it is easily understood in that way than the example below:

if(result)
{
  ....
}

Readability is maintainability.

Solution 4

What can go wrong? Exactly, nothing. In the best case your function does nothing. That's still better then having some random argument be accepted as true.

If you use true all the other time, then you should also check explicitly for it.

While I'm certain in favor of if foo: in Python, there is a big difference here and that is the fact that in the end some random jQuery programmer may come a long and thinks "Oh! It also works with a string and new Boolean()" and uses that.

Concerning Sean's suggestion to use !! it really depends whether you only want a boolean to be accepted or anything that's true. For a clean API I would only accept booleans though.

Solution 5

To add to this conversation, sessionStorage and localStorage variables can only be stored as strings today, so if you use:

sessionStorage.setItem('completed',true);

It will actually be converted to a string and stored as 'true'. I've seen many people writing methods and conversions to manipulate this value so that they can use an if statement using a boolean value, where a simple

if sessionStorage.getItem('completed') === 'true';

would suffice and is perfectly legible and maintainable in my opinion.

View more solutions