My JavaScript book is out! Don't miss the opportunity to upgrade your beginner or average dev skills.

Monday, January 18, 2010

Good Old And Common JavaScript Errors

... I won't write any introduction, but I'll let you comment, OK?

for loops (plus ++i)



for(i = 0; i < 10; i++){}
// global i defined

for(var i = 0; i < something.length; i++){}
// cache the length if it won't change during the loop
// associate once only under certain conditions to speed up

for(var i = 0; i < 10; ++i) {}
// there is absolutely nothing wrong in above loop

Few people told me something like: "doode, if you use ++i at the end you need to write i < 11 'cause i will be 1 inside the loop" ... basically the for loop has been rarely understood, let me try again:

// directly from C language, 1972
for(

// optional inline declaration, coma accepted for more vars
;

// optional condition to verify
// if undefined, it loops until a break is encountered
// this condition is performed BEFORE the first loop
// if false, the loop will never be executed
;

// optional POST operation, it will never be executed
// if the condition is false, "", 0, or null
// it is executed in any case AFTER the loop, if any
){};


// example
for(var i = 0; i < 1; ++i){
// one single execution
// i will be 0 here
};
// i will be 1 here

The reason ++i is preferred over i++, whatever Google go language decided about it, is that i++ and ++i are totally different and you need to understand when, or why, you need i++ or ++i.
While ++i increments directly the variable, returning the variable itself, if numeric, i++ returns a NEW variable with the precedent value AND it increments the i var as well.
Pure JavaScript Example

function Num(value) {
this.value = value;
};
Num.prototype.valueOf = function () {
return new Num(this.value);
};
Num.prototype.toString = function () {
return "" + this.value;
};

var a = new Num(0);
var b = a++;
alert(a); // 1
alert(b); // 0
alert(a == b); // false

// again

var a = new Num(0);
var b = ++a;
alert(a); // 1
alert(b); // 1
alert(a == b); // true

It does not matter if speed speaking, the difference is "not that consistent", it matters that we are programmer, and if two things mean different things, we want to understand them, am I wrong?

Already defined variables, and scopes


Another piece of code you can find around the net, is this:

// something defined globally
var num = 1;

(function () {
// try to set a default
var num = num || 0;
})();

Above code will never work, and it's the ABC about scope and functions.
It's exactly like this:

// something defined globally
var num = 1;

(function (num) {
// now try to find the global num without this. or window. ...
})();

Same is for arguments:

function A() {
// this is actually correct since arguments shares same scope
// no shadowing in this case, thanks to qfox
var arguments = arguments || [];
};

What we need to understand, is that next code is ALWAYS true:

var u = 123;
(function () {
var u = (typeof u === "undefined");
// u is TRUE!!!
})();

The moment we define a scoped variable, it has precedence over whatever outer scope: bear in mind!!!

new Array, new Object


I can't wait the day people will stop to prefer this code:

var a = new Array(); // NOTE: no length/elements specified
var o = new Object(); // NOTE: no object to wrap/convert

rather than:

var a = [];
var o = {};

There are no excuses at all. First of all, the "new Array/Object" way is unsafe, since both constructors can be easily redefined/wrapped, as explained ages ago in many different security related JavaScript issues.
Moreover, we have both more bytes to digest and reduced performance, and a compiler/minifier that is so cool that will change automatically new Array into [] will simply potentially break our code so we cannot even rely improvements with production code.
So ... PLEASE DON'T!

... and new Function ...

Hey, Java guys, please answer this question:
Would you ever use in your Java life a factory method via "new" ???
Seriously, I don't get it ... let's go one step back ...

The Meaning of Factory


The essence of the Factory Pattern is to "Define an interface for creating an object, but let the subclasses decide which class to instantiate. The Factory method lets a class defer instantiation to subclasses."


JavaScript Functions Behavior

If a JavaScript function returns a primitive value such: "string", true, false, null, int, double, new function will return an instanceof that function, overriding the returned result.
Same is for no returns, or return undefined.
If a JavaScript function returns an object, the usage of new is completely redundant and useless.

Array, Function, Object, and RegExp are Factory!!!



var a = Array();
var b = new Array();
var c = [];
[
a instanceof Array,
b instanceof Array,
c instanceof Array
]; // true,true,true


Boolean, Date, Number, and String are both primitive casts and Object constructors!!!



var a = 1;
var b = Number("1");
var o = new Number(1);
a === b; // true
a === o; // false
a == o; // true
a instanceof Number; // false
o instanceof Number; // true
typeof a; // number
typeof o; // object

As summary, and again, please repeat with me:
If a JavaScript function returns a primitive value such: "string", true, false, null, int, double, new function will return an instanceof that function, overriding the returned result.
If a JavaScript function returns an object, the usage of new is completely redundant and useless.


new Class(whatever) === new Factory(whatever)!!!


This is the most misunderstood part that seems so logical but does not make sense at all. 99.9% of cases and in different libraries we can find the classic call to new Class to create what people think is a Class in JavaScript (in few words, to populate the prototype of a runtime created function).
Wait a second ... did I just say: to create a Class?
Uhm ... so, we don't create an instance ... we create a class ... uhm again, so, we are using a Factory, isn't it? :zizi:

Function.createClass() would be hundreds of times more correct, logically speaking, but we all like the new Class, right?
Well, I wonder how many Java Gurus create they Java classes via new Class() on daily basis ... what? You have to define a class, not to create one? ... oh, REALLY???
MooTools use new Class to create runtime functions, parse the configuration object in order to populate the created function prototype, extending it with a completely useless this, that could be simply Class.prototype without useless runtime instanc`es creation for each Class, to finally return a function that is not instanceof Class ... not even primitives values in JavaScript are that ambiguous, isn't it?

new Class({}) instanceof Class; // FALSE!!!

MooTools is simply one in the ocean of libraries that use this misconception to create Functions, not Class, Function.
Considering that JavaScript has no classes, something apparently unacceptable for GWT and friends maniacs that absolutely need to compile their Java application inevitably into a prototypal based programming language as JavaScript is, thinking they have more power over the application, I agree that classic OOP is something so intrinsic in the IT panorama, that it will never die and it could make somehow things simpler for newcomers into JavaScript. OK, I am with you ... can we at least agree about the fact new factoryMethod does not make sense at all?

Logically Speaking ...


Rather than nonsense why don't we simply respect JavaScript native behavior?
If we consider native constructors behaviors such Boolean, Number, or String, I have already described what's up if we use new or not.
There is a massive difference between new and not new ... so why don't use the same behavior for Funciton?

// example
var f = Function("return 'function'");
alert([f instanceof Function, f()]);
// true,function

var f = new Function({toString:function(){return "instance"}});
alert([f instanceof Function, new f()]);
// true,instance

In few words we can easily think about Function as a primitive function creator AND as a function instances creator, those that we'll love to use to instantiate our application objects. Does it sound really that silly?

var Function = (function (Function) {
// (C) WebReflection Mit Style

// Class with private constructor
function constructor(constructor) {
function Class() {
constructor.apply(this, arguments);
};
return Class;
};

// Class creator
function Class(__proto__) {
function Class() {};
if (__proto__) {
if (hasOwnProperty.call(__proto__, "constructor")) {
Class = constructor(__proto__.constructor);
};
(Class.prototype = clone(__proto__)).constructor = Class;
};
return Class;
};

// special Function
function $Function(arguments, body) {
return this instanceof Function ?
Class(arguments) :
body == null ? Function(arguments) : Function(arguments, body)
;
};

// private variables
var
hasOwnProperty = Object.prototype.hasOwnProperty,
clone = Object.clone || (function () {
function clone() {};
return function (__proto__) {
clone.prototype = __proto__;
return new clone;
};
})()
;

// public static explicit factory
$Function.create = Function;
$Function.createClass = Class;

$Function.prototype = Function.prototype;

return $Function;
})(Function);

Here we are, and the best thing is that we can extend the prototype chain simply passing another prototype as configuration object.

function A(){};

var B = new Function(A.prototype);

alert(new B instanceof A); // true

Moreover, since whatever library will use a Function to create other functions, but since most of the time the new Class won't return an instanceof Class, with my suggestion whatever created "Class" will simply inherit Function.prototype.methods OR, if we want to add custom public methods, we can always do it later.
Finally, with a simple closure, we can still be semantic, and define private methods:

var MyClass = new Function((function () {
function set(value) {
this.value = value;
};
return {
constructor:function (value) {set.call(this, value)},
get:function () {return this.value}
};
})());

var o = new MyClass(123);
alert(o.get()); // 123

Now it's your turn, feel free to comment the path I followed, and give me other suggestions/common errors, if you remember any ;)

Update I almost forgot one of the most famous/silly one ...

Arrays "for in"



var a = [];
a[1] = 1;
a[0] = 0;

// global key defined
for(key in a);

// key will be first 1, then zero, in Internet Explorer and Opera
for(var key in a);

Another common mistake is to think that "for in" is good for arrays.
First of all the iteration order is unpredictable, there is no rule about "for in" iteration (or if any, nothing that is standard, cross-browser speaking) plus it's way slower than a normal iteration via length. Please DON'T!!! In this case as well.

28 comments:

Ed Spencer said...

Good stuff, though in your "Array, Function, Object, and RegExp are Factory!!!" section, c instanceof Array is false, not true (try it in firebug). myVar instanceof Object is true for each though. Perhaps your {} was meant to be a [] though.

The "for in" treatment never used to guarantee order in the spec - I believe that has changed though for the new ECMAScript to force iteration in the order that properties were defined on the object.

raz said...

"Considering that JavaScript has no classes, something apparently unacceptable for GWT and friends maniacs"

Haha, I think the reason for GWT is actually that 99.9% of the Javascript "coders", and even the gurus writing libraries commit the mistakes you described.

Great post though.

raz said...

Oh, and the most common, relatively trivial, problem I found is the almost indistinct use of == and ===

kangax said...

There's no int or double value (sic) in Javascript, though.

Primitive values are null, undefined, string, number and boolean. Everything else is an object. No more, no less.

And `var c = {}; c instanceof Array` is not really `true` as in your example. Typo?

I agree that uppercasing factory-like method is misleading.

Andrea Giammarchi said...

Ed, thanks to spot the typo, hell yes, was definitively []
The "for in" behavior is not standard.
As is in PHP, and if you understand how the hash is handled, makes sense to follow the assignment order but Firefox, Chrome, and others do not respect this while IE and Opera do.
At the same time, every index can be defined or re-defined in whatever moment, so why use "for in" that is even 2 up to 5 times slower? ;)

Andrea Giammarchi said...

P.S. and I am not even talking about Array.prototype and possible methods ... where will these be in the "for in", before or after? And do we want to loop these inherited methods as well? I don't think so, it's simply an error.

qFox said...

Nice post! I do have some comments :)

arr.length is already cached by javascript. Putting it inside another variable will only save you the lookup for a property. It's not as drastic as doing it in PHP (count). Eg. for(;i<arr.length;) is marginally slower than for(n=arr.length;i<n;) because of the property lookup you save. Nothing else.

I'm going to have to do some tests about the ++i vs i++ in terms of variable returned. I didnt know one returns a different reference (the for comment regarding this is correct :). I didn't read anything in the spec about this.

The arguments example (next) is either confusing or wrong... see:

var a = function () {
var arguments = arguments || 'moo';
alert(arguments);
};
a();

Arguments, even if empty, will never eval to false. The empty array doesn't either (even though arguments is no array anyways). Arguments is a special object and objects are always true. The other remarks about shadowing still hold though :)

[] and {} are also faster (not sure why) than their constructor parents.

Regarding the return of the function. I'd have to do some testing, but I think you might be confused. The language explicitly defines the behavior of new String vs String, etc. It clearly says What either will do and the suble differences... But I can't say for sure.

Maybe you're confused with the behavior of a constructor that explicitly returns a value? When one does, the returned value is actually returned instead of the object (which you would expect). So:

var A = function(b){
if (b) return "yes";
};

A() === undefined
new A() === object A
A(true) === "yes"
new A(true) === "yes"

Oh and like Array and Object, nobody should use Function in the first place :) It's like using eval().

The forin is slower because it will iterate over all the inherited properties of Array as well...

Sorry for correcting you so much :/ I enjoyed your post! If you're interested in these peculiarities I suggest you start reading the ES5 specification...

Andrea Giammarchi said...

kangax in UK keyboard [] and {} are in the same buttons ... yes, typo, that was obviously an Array, can't get the fuss around that ... while int and double were to describe Numbers to Java guys, but thanks to remember me that typeof "int" does not exists ... :D

Andrea Giammarchi said...

qFox there is nothing in this post I am confused about, but feel freee to test everything I said, it's just like that ;)

Doug said...

Fantastic article! Pointing out that JS functions are factories has cleared up a lot of confusion for me. I also never fully understood why "++i" was preferred over "i++" in "for" loops until now.

The following example produces an error:

var f = new Function({toString:function(){return "instance"}});
alert([f instanceof Function, new f()]);

specifically, "new f()" results in an "instance is not defined" error.

Am I missing something obvious?

Andrea Giammarchi said...

Doug, I guess you missed my Function library/wrapper, kinda mandatory to test that piece ;)

qFox said...

Regarding incremental operators; See Ecmascript5 11.3.1. According to that paragraph the returned value SHOULD be the result of ToNumber(GetValue(expr))+1, which is also stored in the variable before returning.

In other words, whatever browser is doing what you said (they might all), they're not doing it according to the ES5 spec (but maybe the ES3 spec, I haven't checked).

So you should always (at least) assume that ++i and i++ return a reference other than i itself.

The arguments example holds; try this:
(function(a){
var arguments = arguments || [];
alert(arguments[0]);
})(10);

Will alert 10. Couldn't if arguments was in fact "gone". Your statement would be correct if you'd do "var arguments = 5;" because the new var would shadow the arguments, making it impossible for resolution to get it :)

As for the "new" operator, check out paragraph 11.2.2 of ES5. It says it returns the result of [[Construct]]. Now see 13.2.2 for [[Construct]]. The ninth line says "if Type(result) is Object then return result". Now to clarify, Type() is defined in chapter 8.

Basically it comes down to this: If the constructor returns anything but an object, a new object of the constructor type is returned. If the constructor returns any object, that object is returned. Demonstration:

var A = function(){ return 5; };
A.prototype.toString = function(){ return "A"; };
var a = new A(); // instanceof A
alert(a);
var B = function(){ return {}; };
A.prototype.toString = function(){ return "B"; };
var b = new B(); // new empty object, not instance of B
alert(b);

I hope I've done a better job at supporting the statements I made before :) If anything is still unclear, you contact me through twitter :)

And golly, I hope I didn't make any mistakes here myself ;)

qFox said...

Ahw. I did make a small typo. In the last example, the second prototype declaration should be for B, not A. The result is the same though. (Feel free to edit that)

Andrea Giammarchi said...

Thanks for explanation .. you are right, my example with arguments is completely wrong for the same reason they are in the same scope.
Same happens if we have a var a in the global scope and var a = a || 123 after in the same scope.
The shadowing happens only with outer scope, I should have explained/test better that arguments example.
About typeof "object", since null is overwritten by the returned instance, I am not sure what you say makes sense.
About the i++, if it was by reference, you would have same value after the operation, not the old one.
var a = 0;
var b = a++;
b is NOT a, it's a new number.
a is 1 after that.
With ++a you don't have this "problem" cause the number will be exactly the a value.
++a === a
I have used a fake Num constructor without strict comparator to virtually show the logic, since not everybody can get C++, specially JavaScripters ;)

Andrea Giammarchi said...

P.S. qFox, while it's good to know ES5, this post is about Good OLD errors, where ES5 was not even nominated ... this post does NOT consider ES5 but ES3. Just to underline, nothing else.

qFox said...

I understand :) And I haven't checked what ES3 says in this regard. It's however also good to know what ES5 says since that' the future :)

No I really like these posts. Reminds me that I should also write a few more about the many pitfalls of Javascript...

ta2edchimp said...

wowzen! this is one of the best and most valuable discussions in a comments section i've ever read.
thanks a lot for sharing those thoughts...

ELV1S said...

var operator is truly WTF thing.

num = 1;
(function () {
alert(num || 2) // 1, Obviosly
})();

num = 1;
(function () {
var num = num || 2;
alert(num) // 2! WTF?
})();

var has precedence, I know. But why?! Ruby and Python can live with no var at all.

qFox said...

There's actually another interesting bug going on with the incremental operator... I couldn't quite determine whether you noticed that, but when you do n++ and n is not a number, n should be the old number value + 1. Since PutValue(expression, newValue) should be called, this means that the variable should now contain a number (typeof 'number'!), regardless of what it contained before.

So your object example should leave you with a number (regardless of what browsers actually do). If you figured this out, very nice find indeed :)

I'll chalk it up to yet another obscurity... :)

Andrea Giammarchi said...

ELV1S, python has opposite behavior, local is implicit, no need for var.

qFox, yep, I have noticed it, I ws waiting for somebody spotting the hell of piece of code there that is more weird than logic :D

( try to set ++this.value and you'll have 0 and 2 ;) )

qFox said...

I wrote an article about that here: http://qfox.nl/notes/80

Thanks for the inspiration :)

Andrea Giammarchi said...

love the get/set solution, good stuff ;)

Enrico.Simonetti said...

How would you loop through associative arrays / unknown object properties without the for(var key in a)?

Andrea Giammarchi said...

Enrico there is not such thing: unknown Array
the Object.prototype.toString.call(o) === "[object Array]" will always tell you if that unknown object is an array.
Moreover, you are messing up PHP with JavaScript.
In JS (and many others) Array are arrays, you use them as a stack or you are doing something wrong.
Being a stack what is the point to loop over their items via "for in" ? Mess up everything without control neither knowing if "10" is an index or an associative property/method? I don't think so ;)

Seandy said...

Hi Andrea,

I think i know what Enrico was trying to say.

Let's say that we have an array of javascript which has alphanumeric keys. For example:

var car = [];
car['bmw'] = 'BMW';
car['toyota'] = 'Toyota';

How do you loop through the above array, without knowing the keys? As far as I know, only (for in) is able to retrieve the key for each item in the array.

Andrea Giammarchi said...

Seandy, that is an array used as an object and as I have said that is simply wrong, is PHP array concept in JS, avoid it.
But, obviously, if you need an object, "for in" is the way to iterate, so, if you use an instanceof Object, "for in" is again the way. Now, one step back to understand why a stack should be used like that ...

kdk said...

Thanks much! Details aside (I've still much to learn of JS), you just debugged two days worth of work in two paragraphs :-)

Dmitry Pavlov said...

Some to add: 10 Common JavaScript Coding Errors | Toptal