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.

Answer 1

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.

Answer 2

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