Redux: Why You should Always Use ===

Two weeks ago, my post, Why You Should Always Use === and Other Bad Practices, generated a tremendous amount of discussion. I got a lot of flack for basically everything in the post, but getting torn apart is the best way to grow, right? This post, is intended to act as a tl;dr of some of the feedback and to go into more details about some things I intentionally glossed over. If you have not read the original post take 5 minutes to read it now.

Ready?

I wanted to start with an apology because the title of the original post was terrible. A better title would have been Why You Should Always Use === and Fixing Some Bad Practices. Instead I gave the false impression, just based on the title, that using === was a bad idea. Perhaps this was why the post was so popular on reddit, but this is the opposite of what I was trying to say. In fact, I believe almost all implicit type conversion is a bad idea, and this was the major point of my original post. In the original code I shared, the author was relying (either intentionally or not) on an implicit conversion masked behind a variable name. In the original code, index was a string, but it was being compared to an integer. This means the value of index (a string) was being converted to a number implicity before comparision (the same as '0' == 0). Relying on this feature is simply a maintainability nightmare.

Secondly, having a comment and then a dummy console.log statement threw a lot of people off. So I want provide some updated code to fix this issue for this post. Without further-ado, the code we should have started with two weeks ago:

if(attributeArray instanceof Array) {  
    for(var index in attributeArray) {
        if(index == 0) {
            // do something with attributeArray[index]
        } else {
            // do something else with attributeArray[index]
        }
    }
}

In my original post, I cleaned this code up because it has several issues. Rather than going over these issues again, I want to skip to the final code from the first post and go from there:

if(attributeArray instanceof Array) {  
    for(var index=0, len=attributeArray.length; index < len; index++) {
        if(index === 0) {
            // do something with attributeArray[index]
        } else {
            // do something else with attributeArray[index]
        }
    }
}

So with this code in mind, I first want to look at one of the antipatterns in the above code that I intentionally ignored in the original post: the instanceof Array expression.

instanceof Array is an Antipattern.

Using instanceof Array is indeed considered an Antipattern.

Why?

Mozilla Developer Network explains it best:

Different scope have different execution environments. This means that they have different built-ins (different global object, different constructors, etc.). ... For instance, [] instanceof window.frames[0].Array will return false, because Array.prototype !== window.frames[0].Array and arrays inherit from the former.

For many people, this subtle issue will not be a problem because their code does not utilze multiple windows or frames.

For those that do have javascript across multiple windows or frames, a common workaround to this issue is to call the Object.toString() and see if it is '[object Array]'. Because you can modify an object's methods, or its prototype methods a safe way to accomplish this is with the following comparision:

Object.prototype.toString.call(myArray) === '[object Array]';  

However, if you are using ECMAScript 5 there is no point in performing this check yourself. Instead, you get the same safety by relying on the Array.isArray() method. Since there is no reason not to use this method and I have no idea how my code could be used in the future, I'm going to be defensive and include the most effective isArray() check:

if(Array.isArray(attributeArray)) {  
    for(var index=0, len=attributeArray.length; index < len; index++) {
        if(index === 0) {
            // do something with attributeArray[index]
        } else {
            // do something else with attributeArray[index]
        }
    }
}

Now I want to revist one of the arugments I originally made for ditching the for-in loop: SPEED.

Using index++ is Slow?

There is a body of emperical evidence, based on microbenchmarks, that says using post-increment is marginally slower than pre-increment.

This may be true, but in practicality the differences are going to be so miniscule, it probably does not matter.

In fact, a microbenchmark says index += 1 is faster than both index++ and ++index. Since we have no reason not to make this minor change, our new code is as follows:

if(Array.isArray(attributeArray)) {  
    for(var index=0, len=attributeArray.length; index < len; index+=1) {
        if(index === 0) {
            // do something with attributeArray[index]
        } else {
            // do something else with attributeArray[index]
        }
    }
}

Microbenchmarks Are Worthless.

The best feedback I got to my original article was a guy telling me the jsPerf benchmarks were (almost) worthless.

Most of the benchmarks are worthless because of modern javascript compiler optimizations. Modern javascript engines perform a number of JIT optimizations that naive jsPerf benchmarks can mask.

The issue is such a huge deal, Vyacheslav Egorov, a v8 engine developer has led a personal crusade against microbenchmarks. One of the best talks I've seen so far this year is his talk entitled Performance and Benchmarking. That talk is really enlightening and it will definitely change your opinion on JavaScript benchmarking. Egorov also has an incredible blog where he talks about v8 internals and benchmarking in great detail.

The final example checks if index===0 every iteration.

This criticism is a fair one. The code snippet can easily be reduced to avoid this unneed check with every iteration:

if(Array.isArray(attributeArray)) {  
        if (attributeArray.length >= 1){
            // do something with attributeArray[0]
            for(var index=1, len=attributeArray.length; index < len; index+=1) {
                // do something different with attributeArray[index]
                }
        }
}

You should focus more on readability than micro-optimzations

Another fair criticism. One of the ways we can reduce the complexity of the code and make it more readable is to use ES5's Array.prototype.forEach():

if(Array.isArray(attributeArray)) {  
    if(array.length > 0) useArrayElem(array[0]);
    array.slice(1).forEach(useElemDifferently);
}

One of the biggest arguments against using Array.prototype.forEach() is that it is slower than a regular old for loop.

This is most certainly true. The slice() call makes a copy of the array, so this results in inefficiencies. A regular for loop also does not have to worry about invoking a function and dealing with stack-frames, so more inefficiencies are introduced here.

Since I have no compelling reason to write the code in such a manner, the final version of the code in production is going to be the θ(n) version shared above:

if(Array.isArray(attributeArray)) {  
        if (attributeArray.length >= 1){
            // do something with attributeArray[0]
            for(var index=1, len=attributeArray.length; index < len; index+=1) {
                // do something different with attributeArray[index]
                }
        }
}

Whoever throught that such a short snippet could elicit such discussion? Read the rest of the comments on /r/programming and Hacker News. They were very educational and worth taking the time to go through too.