JSLinting existing JavaScript and eqeqeq

Introduction

As I've previously mentioned JSLint is a tool by Douglas Crockford that checks code against various rules in order to find code that could potentially be buggy or ambiguous. One such rule is that users should not use two equals (==) but should use three (===). There is an option that turns on whether it complains when comparing variables (x==y) but JSLint will always complain when comparing against system constants like "", null, undefined and 0. This because when using two equals the values are coerced between types, so we get the following surprising results (all of the if's are true).

var result = "";

if  (undefined == null) {
    result += "true,";
} else {
    result += "false,";
}

if  ("" == false) {
    result += "true,";
} else {
    result += "false,";
}

if  (0 == false) {
    result += "true,";
} else {
    result += "false,";
}

if  (0 == "0") {
    result += "true,";
} else {
    result += "false,";
}

if  (undefined != false) {
    result += "true,";
} else {
    result += "false,";
}

if  (!undefined == true) {
    result += "true";
} else {
    result += "false";
}

alert("result is "+result); // true,true,true,true,true,true

JSLint will give errors for comparing to true, false, "", 0, null and undefined if we use coercion. This is to stop the programmer being caught out when they may not remember all the rules. Compare the results when we add in extra equals signs...

var result = "";

if  (undefined === null) {
    result += "true,";
} else {
    result += "false,";
}

if  ("" === false) {
    result += "true,";
} else {
    result += "false,";
}

if  (0 === false) {
    result += "true,";
} else {
    result += "false,";
}

if  (0 === "0") {
    result += "true,";
} else {
    result += "false,";
}

if  (undefined !== false) {
    result += "true,";
} else {
    result += "false,";
}

if  (!undefined === true) {
    result += "true";
} else {
    result += "false";
}

alert("result is "+result); // false,false,false,false,true,true

Which is at least easier to predict. So the question I want to ask in this blog post is not to go into the intricacies of JavaScript type coercion but to look at what we should consider when we are making old code pass JSLint.

Situations in non lint passing code

Perhaps the most common situation to come across in old code is

if  (x != null) {
   // do something
}

Now to make this pass, we could just convert it to what this actually means.

if  (x !== null && x !== undefined) {
   // do something
}

Which is pretty ugly. But we can take advantage of the fact the following

if ({}) {
    //true
}

if ([]) {
    //true
}

if  ("string") {
    //true
}

if  (45) {
    //true
}

if  (null) {
    //false
}

if  (undefined) {
    //false
}

if  ("") {
    //false
}

if  (0) {
    //false
}

All objects, arrays, strings that are not empty, numbers that are not 0 all evaluate to true when we evaluate them (despite them not coercing, so [] == true is false).

So going back to our original code, we can first ask is 0, false or "" a truthy value for the variable? If the variable is an array or object, we immediately know it is invalid to be these values, so we can instead make our legacy code pass JSLint by just doing an if.

// x is an array or object so we don't need to test against 
// anything as this will only pass if it is initialised
if  (x) {
   // do something
}

If the variable is a Number, String or Boolean though we need to be very careful. Is it obvious if "", 0 or false are invalid values? It is probably safer to go back to comparing against null and undefined.

You might think that for numbers we could use isNaN - you'd be wrong.. isNaN(null) is true. Casting null to a number produces 0. So how else can we determine if something is a number, a string or a boolean and not null or undefined?

if  (x !== null && x!==undefined) {
   // do something
}

if  (x && x !== 0) {
   // do something
}

if  (typeof(x) === "string") {
   // do something
}

I like the last one best, because it is really specific about what it is testing for, but what I don't like is that it has a string constant and it is quite open to programmer mistakes...

if  (typeof(x) === Number) {  // Very bad code, do not copy
}

if  (typeof(x) === "Number") {  // Very bad code, do not copy
}

if  (typeof(x) === "int") {  // Very bad code, do not copy
}

if  (typeof(x) === "number") {
   // could still be wrong if we want "3" to pass
}

So, what about having functions to work out if something is a number, string, bool? I wouldn't recommend it in speed sensitive code, but the following code would make your if clearer. If you were using the Google Closure compiler though, you might find that it automatically inlined the code from the functions and relied on gzip to reduce the extra size of those string literals.

is = {
  number : function(number) {
    return typeof(number) === "number";
  },

  numberCoercible : function(number) {
    return typeof(number) === "number" || number == Number(number);
  },

  string : function(string) {
    return typeof(string) === "string";
  },

  stringCoercible : function(string) {
    return typeof(string) === "string" || string == String(string);
  },

  boolean : function(boolean) {
    return typeof(boolean) === "boolean";
  },

  booleanCoercible : function(boolean) {
    return typeof(boolean) === "boolean" || boolean == Boolean(boolean);
  }
};

I decided to create separate functions for the coercing to make it more obvious what it is checking for at each if. They don't call each other so you only have one method call per if and so there is no way a developer can pass something like isBoolean(bool, true) to signify they want coercion - it might be worse than the original in terms of being clear in meaning. The coercion test is done by comparing the value passed into the type cast with the value itself. For example...

  • String(null) is "null" which is not == to null.
  • Number(null) is 0 which is not == to null.
  • Number(undefined) is NaN which is not == to undefined
  • Bool(2) is true which is not == to 2
  • Bool(undefined) is false which is not == to undefined
  • etc.

However, we do get some slightly strange results, like "" being coercible to false and [] being coercible to string (["1"] == "1" and ["2","3"] == "2,3"). I would think that the use case for using string and boolean coercion would be quite small and possibly best left off the if class. We do at least get false if null or undefined are passed into these functions.

Conclusion

Whether you use the global functions or not (I haven't decided myself whether its worth the payout performance and size layout when not using Google Closure) I hope this post underlines that == vs === is difficult and care needs to be taken in writing and changing code when testing for null or undefined.

MORE BY LUKE

Seven Surprising JavaScript 'Features'

Aurelia, less2css and bundling

blog comments powered by Disqus