Double Trouble: Two risky features come together for a conflict in JavaScript

On the value of being careful with modifying global scope. There is a code conflict I encountered recently when combining somebody else's JavaScript code with a 3rd party library. It didn't go well, and took time code-digging / stepping-through / debugging / tracing, until finally the cause became apparent.

JavaScript in general is very flexible (beyond loose), which is reason to both love it (“With great power…”) and hate it (“…comes great responsibility”). But blaming one's tools isn't helpful when you need to get a job done.

These are the two things that caused the conflict. Both are discouraged, but you won't understand why until you see them together.

Array.prototype.[method]

There is a very powerful ability in JavaScript to modify a prototype (a sample Object template similar to class in OOP languages) by adding new methods:

MyClass.prototype.newMethod = function() {
   // New method for "class" MyClass
}

Then you can call this method on any object of MyClass. Sweet.

Yet what then seems extra-powerful, is the ability to do the same with system classes. And, since (almost) everything is an object in JavaScript, you can add your own methods to String, Date, RegExp. This could really help locally improve readability of your code, since instead of getPresidentNames(getAllNames()) you could call getAllNames().getPresidentNames(), potentially chaining operations instead of matrushka-dolling them.

But modifying system objects like this is discouraged, because the change is global. Your added method will be visible to all of your code (which, in our day and age, 90% is made of code not written by you).

In my case, Array object was expanded with .max() and .min() methods. This sounds useful, but…

Array property iterator

Here's another convenient feature—you can iterate over all of the object's properties:

for(var prop in myObject) {
   console.log("myObject has property " + prop);
}

Very powerful, though potentially dangerous. If you run this in a controlled environment, where every single line of code was written by you, you'll get the list of properties that you've yourself defined. It is a very nice and expected result.

But what if someone redefined the object elsewhere?

Custom objects—not a problem, especially if you use closures.

System objects—not a problem… until somebody decides to add a method. Then you get list of properties and methods, including the newly added one.

For arrays, iterating with for(… in …) goes through the list of existing array indices. If you add methods—it will include them too. E.g. the list of values will be "1", "2", "newMethod".

The painful part (and that makes it hard to detect these kinds of issues) is that even if you make the change inside a closure, you're still modifying a global object, so you'll affect all of your code. Here's a simple Fiddle demonstrating this:

What should we do?

Two options.

None of the above features are really damaging by itself. Your code will work fine if you do either. So, when a conflict happens, you can play the blame game (Don't you expand prototypes! No, don't you iterate over arrays as objects!).

But that's not very productive.

On the other hand—you can understand that both practices are risky, since you don't always have control over all the code. So just avoid these.

For methods on global objects it is probably safer to use functions, if you really need to. They don't look as nice, but they are local to your closure, so—safe.

For iterating over arrays, there are other methods (for(i = 0; i < Array.length; i++), or Array.forEach()), whatever suits you. Or, if you are so adamant on using for(… in Array), then make sure to type-check to exclude functions (at least!).

You'll figure it out :)