Tuesday, February 16, 2010

JSLint: The Bad Part

Every programming language has somehow defined its own standard to write code. To be honest, as long as code is readable, clear, and indented when and if necessary, I think we do not need so many "code style guides" and, even worst, sometimes these "code standards" let us learn less about the programming language itself, helping if we are beginners, sometimes simply annoying if we are professionals.

Disclaimer

This post aim is absolutely not the one to blame one of my favorite JS gurus, this post is to inform developers about more possibilities against imperfect automation we'll probably always find in whatever excellent spec we are dealing with. This post is about flexibility and nothing else!

JSLint And Code Quality

Wehn Mr D says something, Mr D is 99.9% right about what he is saying ... he clearly represents what we can define a Guru without maybe or perhaps but he, as everybody else here, is still a programmer, and we all know that every programmer has its own style if we go down into details.
Mr D programming style has been historically summarized in this Code Conventions for the JavaScript Programming Language page.
All those practices are basically what we can find out about our code using a well known and widely adopted parser: JSLint, The JavaScript Code Quality Tool.
I write JavaScript and ActionScript (based over the same standard) since about 2001 and generally speaking, as experienced developer, I trust what I meant to do, and rarely what an automation tool supposes to teach me about the language.
This is like studying remembering rules rather than being able to understand them, something surely academical, nothing able to let us explore and discover new or better solutions ... we are stuck in those rules, but are we machines? Are we Mr D code style clones?
This post is about few inconsistent points regarding JavaScript Code Quality, with all due respect for somebody that simply tried to give us hints!

Indentation

The unit of indentation is four spaces. Use of tabs should be avoided because there still is not a standard for the placement of tabstops
While size always matters, since to move 1Gb or 600Mb in a network still makes a big difference, I wonder what's wrong with just two spaces. The monospace font we all use to write code on daily basis is "large enough", and while 2 spaces rather than 4 are extremely easy to spot, 4 spaces are absolutely ambiguous.
How many times we had to check via cursor if some other editor placed a tab rather than 4 spaces there? Ambiguity is something that does not match at all with the concept of Code Quality, am I wrong?
Finally, while everybody in this world has always used innerHTML due its better performances against the DOM, Mr D. tells us that tabs are not defined as a standard ... have we never heard something like de facto standard?
Specially in a unicode based language as JavaScript is, where tabs are indeed replaced by "\t" even in Mr D JSON specs, how can we think about whatever JavaScript IDE or Engine unable to understand tabs? Let's avoid them, but still, at least let's replace them with something that does NOT occupy exactly the same space!

Variable Declarations

All variables should be declared before used. JavaScript does not require this, but doing so makes the program easier to read and makes it easier to detect undeclared variables that may become implied globals.
OK, got the point, minifier or munger will take care about this so it could make sense. Now, everything is fine to me, as long as I don't read the next immediate point:
The var statements should be the first statements in the function body.
... are we serious here?
So, if var declaration is the first thing I should do, why on earth I should spend 10 times my time to write semiclons and var again?

// first thing to do
var a = 1;
var b = 2;
var c = 3;
var d = {};

// AGAINST
var a = 1, b = 2, c = 3,
d = {}
;

Faster to type, easier to read, I can even group blocks of variable declaration to define different types (primitive first, object after, undefined later if necessary) and thanks to indentation, the precedent point, I must be an idiot to do not understand what the hell I wrote. Is it just me?
I prefer to perfectly know the difference between comma and semicolon and these two buttons are in a different place, there is NO WAY I coul dmake a mistake unless I don't know the difference ... but in that case I guess we have another problem, I know nothing about JS!
Furthermore, something kinda hilarious to me:
Implied global variables should never be used.
OK, assuming for whatever reason we don't consider global functions references/variables, we should forget:
  • undefined
  • window
  • navigator
  • document
  • Math
  • jQuery (dollar $ is global baby!)
  • everything else that supposed to be reached on the global scope
I may have misunderstood this point so I do hope for some clarification, but again, the difference between a local scoped variable and a global one should be clear to everybody since JSLint cannot solve anything, and I'll show you later.

Function Declarations

I agree lmost everything about this chapter, but there are surely a couple of inconsistencies here as well.
There should be no space between the name of a function and the ( (left parenthesis) of its parameter list. There should be one space between the ) (right parenthesis) and the { (left curly brace) that begins the statement body.
...
If a function literal is anonymous, there should be one space between the word function and the ( (left parenthesis). If the space is omited, then it can appear that the function's name is function, which is an incorrect reading.
Let me guess: if there is a name, there must be a space to identify the name, if there is no name, the must a space as well? A function called function which is a reserved word?
I am sure Mr D has much more experience than me, but I wonder if that guy that wrote function function() {} has been fired, is in the Daily WTF website, it is part of the shenanigans group, or if it is still working beside Mr D ... in few words, how can be no space and a bracket ambiguous?

var f = function(){};
I want to honestly know who is that programmer able to confuse above code ... please write a comment with your name and your company, I will send you a Congratulation You Are Doing Wrong card ... I'll pay for it!!!
Same is for the space after the right parenthesis, as if an argument could accept brackets so that we could be confused about the beginning of the function body, isn't it?
Some bad code there in this chapter as well, which let me think truly weird stuff:

walkTheDOM(document.body, function (node) {
var a; // array of class names
var c = node.className; // the node's classname
var i; // loop counter results.
if (c) {
a = c.split(' ');
for (i = 0; i < a.length; i += 1) {
...

So we have to declare every variable at the beginning, included variable used for loops, the i, so the day engines will implement a let statement we'll have to rewrite the whole application?

var collection = (function () {
var keys = [], values = [];

return {
....

wait a second, consistency anybody? How come the next paragraph uses a natural assignment as that one while the Code Quality is to do not use it?

Names

Names should be formed from the 26 upper and lower case letters (A .. Z, a .. z), the 10 digits (0 .. 9), and _ (underbar). Avoid use of international characters because they may not read well or be understood everywhere. Do not use $ (dollar sign) or \ (backslash) in names.
I am not sure if Prototype and jQuery guys shouted something like: epic fail but I kinda laughed when I read it. It must be said that these practices are older than recent framework, and this is why I have talked about do not explore code potentials at the beginning of this post.
Do not use _ (underbar) as the first character of a name. It is sometimes used to indicate privacy, but it does not actually provide privacy. If privacy is important, use the forms that provide private members. Avoid conventions that demonstrate a lack of competence.
So if I got it right, if we identify private variables inside a closure, where these variable actualy are private, we should not identify them as private so we can mess up with arguments, local public variables (remember cached vars?) and everything else ... ambiguous!

Statements

Labels
Statement labels are optional. Only these statements should be labeled: while, do, for, switch.
...
continue Statement
Avoid use of the continue statement. It tends to obscure the control flow of the function.

Labels are fine, continue, which is basically an implicit label similar to goto: nextloopstep should be avoided. But as far as I read the return statement able to break whatever function in whatever point has no hints about being only at the end of the function bosy as is for ANSI-C programmers?

Block Scope

In JavaScript blocks do not have scope. Only functions have scope. Do not use blocks except as required by the compound statements.

with (document) {
body.innerHTML = "is this a scope?";
}
Never mind, somebody in ES5 strict specs decided that with statement is dangerous ...

=== and !== Operators.

It is almost always better to use the === and !== operators. The == and != operators do type coercion. In particular, do not use == to compare against falsy values.
This is the most annoying point ever. As a JavaScript developer, I suppose to perfectly know the difference between == and ===. It's like asking PHP or Ruby people to avoid usage of single quoted strings because double quoted are all they need ... does it make any sense?

// thanks lord null is falsy as undefined is
null == undefined; // true
null == false; // false
null == 0; // false
null == ""; // false
null == []; // false
null === null; // true, this is not NaN

In few words, as I have said already before, null is == only with null and undefined, which means we can avoid completely redundant code such:

// Mr D way
if (v !== null && v !== undefined) {
// v is not null neither undefined
}

// thanks JavaScript
if (v != null) {
// v is not null neither undefined
}

Moreover, since undefined is a variable it can be redefined somewhere else so that the second check could easily fail ... and where is security in this case?

// somewhere else ..
undefined = {what:ever};


// Mr D way
if (v !== null && v !== undefined) {
// this is never gonna happen AS EXPECTED
}

Please Mr D whatever you think about this post, think more about that silly warning in JSLint: it's TOO ANNOYING!!!
One last example about ==, the only way to implicitly call a valueOf if redefined:

if ({valueOf: function(){return !this.falsy;}} == true) {
// hooray, we have more power to deal with!
}


eval is Evil

The eval function is the most misused feature of JavaScript. Avoid it.
eval has aliases. Do not use the Function constructor. Do not pass strings to setTimeout or setInterval.

aliases? It seems to me that eval is misunderstood as well since there is no alias fr eval. This function may be evil specially because it is able to bring the current scope inside the evaluated string.
Function and setWhaatever do not suffer this problem, these global function always ignore external scope.
Moreover, it is thanks to Function that we can create ad-hoc, runtime, extremely performer functions thanks to dynamic access to static resolution:

function createKickAssPerformancesCallback(objname, case, stuff, other) {
return Function(objname, "return " + [objname, case, stuff, other].join("."));
}

Where exactly is the eval here and why we should avoid such powerful feature when we are doing everything correct?

Unit Test IS The Solution

If we think we are safe because of JSLint we are wrong. If we think we cannot do mistakes because of JSLint we are again wrong. JSLint could help, as long as we understand every single warning or error and we are able to ignore them, otherwise it won't make our code any better, neither more performance killer, surely not safe.
There are so many checks in JSLint that somebody could simply rely in this tool and nothing else and this, for the last time, is wrong!
Next part of the post is about a couple of examples, please feel free to ask more or comment if something is not clear, thanks.

JSLint: The Bad Part

All these tests are about possibly correct warnings, often useless on daily basis code. Please note all codes have been tested via The Good Parts options and without them.

Conditional Expressions

I twitted few days ago about this gotcha. An expression to me does not necessary require brackets, specially it does not require brackets when it is already inside brackets.

"use strict";
var lastValidArgument;
function A(b) {
// I mean it!!!
if (lastValidArgument = b) {
return "OK";
}
}

Somebody told me that if we put extra brackets, as JSLint suggests, it measn that we meant that assignment, rather than a possible missed equal.
First of all, if == is discouraged in favor of === how can be possible developer forgot to press the bloody equal sign three times? It does not matter, the inconsistency here is that if I properly compare there two variables leaving there brackets, theoretically there to let me understand I am doing an assignment rather than comparing vars, nothing happens:

"use strict";
var lastValidArgument;
function A(b) {
if ((lastValidArgument === b)) {
return "OK";
}
}

I would have expected a warning such: Doode, what the hell are you doing here? You have superflous brackets around! ... nothing, inconsistent in both cases, imho.

Unused variable

A proper syntax analyzer is truly able to understand if inside a whole scope, we used a variable as we meant to do. Unfortunately, here we have a totally useless warning/error about an extremely silly, but possible, operation. To avoid the error:

"use strict";
(function () {
var u;
// we forgot to assign the "u" var
// how can this if remove such error from this scope?
if (u) {
}
}());

Agreed, u has been used, but why on earth I have no warnings about a completely pointless if? As far as I understand, variables are truly well monitored ... this requires an improvement ... or remove the Unused variable check since in this case it does not change anything.

Pattern to avoid global scope pollution

The new operator is powerful, but it could be omitted and JSLint knows this pretty well! We, as developers, are able to modify behaviors, using functions duality to understand what's going on. Example:

"use strict";
function A() {
if (!(this instanceof A)) {
return new A();
}
}

// always true
A() instanceof A;
new A() instanceof A;

Cool, isn't it?
Problem at line 7 character 9: Missing 'new' prefix when invoking a constructor.
How nice, thanks JSLint!

Look behind without look forward

More about new, who said this requires parenthesis? As soon as I write new I am obviously trying to create an instance. Since the constructor will be exactely the one referenced after, and since after this operation there will be a semicolon, a comma, a bracket (return without semicolon), how can it be possibly a problem for JavaScript?

var a = new A;

If I don't need arguments, I feel kinda an idiot to specify parenthesis ... maybe it's just my PHP background, isn't it?

class A {}
$a = new A; // OK
$b = 'A';
$a = new $b; // still ok
$a = new $b(); // WTF, $b is a string

// oh wait ...
$b = 'str_repeat';
$b('right', 2); // rightright


Strictly Compared

I have already talked about null and == feature, now let's see how dangerous can be JSLint sugestion:

"use strict";
function pow(num, radix) {
return Math.pow(num, radix === null || radix === undefined ? 2 : radix);
}

// somewhere else, since not only eval is evil
[].sort.call(null)["undefined"] = 123;

Above code will both pass JSLint and always fail the check || radix === undefined since undefined will be 123 primitive.
You know what this mean? Unless we do not define an undefined local scope variable for each function (the most boring coding style ever, imho) we should simply avoid === undefined checks, these are both unsafe, these slow down code execution since undefined is global and requires scope resolution, and these are a waste of bytes

function pow(num, radix) {
return Math.pow(num, radix == null ? 2 : radix);
}

That's it, there is no value that could mess up that check: undefined, null, or a number, 0 included!

Nothing bad in this code

This is the best way I know to create a singleton, or a prototype, to have a private scope as well, shadowing the runtime constructor:

"use strict";
var Singleton = new function () {
// private function (or method)
function _setValue(value) {
// lots of stuff here
_value = value;
}
var _value;
this.get = function () {
return _value;
};
this.set = function (value) {
if (!_value) {
_setValue(value);
}
};
this.constructor = Object;
};

Here we have a Jackpot:
Error:
Problem at line 4 character 14: Unexpected dangling '_' in '_setValue'.
Problem at line 6 character 9: Unexpected dangling '_' in '_value'.
Problem at line 6 character 9: '_value' is not defined.
Problem at line 8 character 9: Unexpected dangling '_' in '_value'.
Problem at line 10 character 16: Unexpected dangling '_' in '_value'.
Problem at line 13 character 14: Unexpected dangling '_' in '_value'.
Problem at line 14 character 13: Unexpected dangling '_' in '_setValue'.
Problem at line 2 character 17: Weird construction. Delete 'new'.
Problem at line 18 character 2: Missing '()' invoking a constructor.

We can nullify the Singleton variable itself, but still nobody else can have access to that scope. This is what I call a variable or method private, and I mean it!
Underscore is perfect as first character, since it helps us to understand a "protected method/property", which is never truly protected, and a real private method/property, shared across instances if it's a variable, usable as private method if called via a public one.
The best one ever, in any ase is:Weird construction. Delete 'new'. ... Weird what? A lambda based language cannot create runtime via lambdas instances?

Conclusion

I hope this post will help both Mr D. and developers, the first guru to improve JSLint syntax checks, while latter people to understand errors, being able sometimes to completely ignore them.
I am looking forward for some feedback or, even better, something I did not consider or some example able to demonstrate or underline my points, thanks for reading.

32 comments:

MRoderick said...

I am sure that one Douglas' intentions for these guidelines, was to aid newcomers to the language adopt some good practices, and provide an automated tool to guard against some of these.

Most of these warnings/opinions can be configured on a file by file basis.

So, until we have ES5 capable browsers that understand strict mode and actually bitch about it, I am quite happy continuing to use JSLint to assist me and the teams I work with.

Andrea Giammarchi said...

MRoderick this is exactly the idea of this post. Inform developers, newcomers as well, that not everything is gold there, and a tool cannot automatically make us skilled.
As I have said, it's not against Douglas purpose, hints ;)

edvakf said...

Hi,
Unrelated to the most of this post, but I wondered why you can get the global object with
[].sort.call(null)

Andrea Giammarchi said...

the window object has a length property. The sort method returns the array itself.
var a = [];
alert([].sort.call(a) === a);

If we use call or apply with null or undefined context, the engine will assume it's the global one (should disappear in ES5 via "use strict").
In few words sort context will be the window and it will think window with a length is an Array. It returns it, and voila' the window object :)

Jordan Boesch said...

I understand where you're coming from. It can be a little frustrating when you run your code through JSLint and you find a bunch of silly "errors".

For more experiences developers, JSLint is there to give us friendly reminders about our code, by no means is it the "gospel".

For instance, there will be cases where you meant to make a comparison using == instead of ===. I think that it's really up to your best judgment for your own code and what you want it to do.

Overall, I think JSLint does a good job making people aware for some generally good practices and it's a good starting point for beginners.

Andrea Giammarchi said...

Jordan totally agreed. I've been more than sarcastic but you summarized what I meant. As you said, it's about flexibility and understanding, but with Aptana non updated JSLinte plugin, errors are truly annoying when you have to commit while warnings are less stressful and a good reminder.
Classic error I can't cope with anymore:
// inside a closure
var ShortCut = my.name.space.ShortCut = {
// properties here
};

I do hope Aptana IDE will update the plugin ASAP since indeed latest JSLint does not recognize it as an error.

qFox said...

The variable hints have to do with the fact that they are parsed ahead of function invocation. It is to prevent coders from conditionally defining vars and assuming they are only created if that branch is used.

And I think "D" also advocates only using a single var list statement... :)

I do agree that some warnings of JSLint should also be disable-able. I personally hate the warning for this line

if (x) return y;

Bitching about expecting if (x) { return y; } ... :/

Now, take a deep breath :p

MRoderick said...

Andrea: I would recommend that you run your JSLint with something like Juicer, which fetches the latest JSLint on install/update.

http://github.com/cjohansen/juicer/

Also, I would recommend to ANYONE using JSLint, to take the time to understand all of the flags that can be set.

None of my files use the default settings, I don't agree with Douglas on many points, and different teams also have different opinions when channeling Crockford :-P

Brett said...

Great blog title...

I wish tabs were just accepted as being 4 spaces, and forget the bandwidth-hogging, anti-environment energy consuming, extra-space engendering, 4-space rule and go solely with tabs. (While we're at it, let's try to get GPL to allow a URL instead of source code inclusion.)

I can't stand the limitation against switch case fall-throughs (even commented ones or same-line ones), as to me that is the big advantage of using a switch.

I do believe in always using brackets, except in your scenario with parentheses. Too many times one needs to expand the construct and forgets about the brackets or gets lulled into thinking an indent indicates the structure. But even here, I wouldn't want to be too dogmatic in imposing it on others.

For the space between a function and name, I disagree with this for a different reason; I want to distinguish between invocations and function definitions for the sake of convenience in searching for one or the other through documents.

Wholeheartedly agree with your rejection of the prohibition of the underscore. Even for non-closures, since JavaScript doesn't really offer true instance privacy across a class (except with the memory-management-requiring Relator that you blogged about earlier), it's certainly better to tell others "hands off" than to say "hands up" to the problem.

I love JSLint as a tool, but I hope it can be made more configurable so that we are not corralled into a new dogma which can be nearly as bad as sloppy code. But it's easier to complain than it is to submit patches. :) It's thankfully already fairly configurable but there are a few areas it needs more flexibility.

Btw, I saw one site claim JSLint was BSD, but it just looks merely copyrighted to me. I'd really rather support an open source tool which we actually could fix and redistribute if we wished. For example, I made a suggestion earlier about suggesting "]]>" sequences (e.g., arr[arr2[3]]> 5) be flagged as they could interfere with script tags in XML documents (interpreted as the closing of a CDATA section), and since Doug just felt scripts shouldn't be done inline (though I usually agree with this), I felt unnecessarily constrained.

Andy Tjin said...

There is a very good reason for having something *like* JSLint, when working with large groups of developers with different levels of JavaScript expertise. Most of your statements are valid when you are working in small projects with JS-developers on the level of your definition of a guru. One of the things you will benefit from in ongoing/longterm projects is style/syntax consistency and readability of code.

I agree that Douglas' old recommendations might be outdated now. (Like the function-space-rule, the one-var-statement-per-declaration-rule, underscore-rule.)

I also agree that there are some JSLint features that should be optional, which are not right now. I would suggest to simply write a better JSLint that fits your environment.

I would use it :-)
Andy

P.S. There is one reason why you should be careful with creating dynamic functions with Function. The JIT-compilers of these days will not be able to pre-optimize code created in there. So you'd want to keep the code of the generated function as small as possible.

P.P.S. You will not believe how often I have found code with jslint that said "if (a = b)" when this was clearly NOT meant.

Jani Hartikainen said...

Here's another reason to not have all your vars on top of the function:

I think it was pointed out in Code Complete, that you should declare variables close to where they are used. They had some fancy word and explanations for it that I can't quite recall at the moment, but the gist was that it makes it easier to find the relevant parts.

Andrea Giammarchi said...

Thanks Andy, and interesting point about JIT pre optimization.
Anyway, to be honest that Singleton strategy is not common at all and there are no real benefits over a classic:
var Singleton = (function(){
// private scope stuff
// ...
// the singleton
return {
methodName:function () {
_pvt.call(this);
}
};
}());

Do you think a runtime executed closer could have some side effect as well for JIT compilers?

a said...

Andrea, I think the 'implied globals' Doug is talking about are variables defined inside a function without the 'var' keyword, which implicitly makes them global. His proposed solution to this is to always use var inside functions and if you need to define a global variable explicitly say so, e.g. by attaching it to the window object. So you can use existing globals like document, just being careful not to accidentally create new ones.

Andrea Giammarchi said...

window is browser dependent, there is not such thing in server side JS. "this" is not the global object anymore via "use strict"; directive, so we are trapped ^__^;;

glathoud said...

Hello Andrea, I also feel in some cases frustrated by JSLint and its "simplified Javascript" and styling restrictions.

Here's a *full* ECMAScript 3 code checker that does not impose any styling restriction. I hope this is of any interest for you.

John said...

Any aid to producing good code is worthwhile. The author of this article obviously doesn't like JSLint and decided that a rant against it is a good topic.

To repeat, any aid in the production of good code is worthwhile. You don't have to use it. If you don't like it, ignore it. But you won't keep a job in my team if your code is not up to the standard I expect from my people.

All the author is doing is merely substituting HER ideas of what makes good code for those of the author of JSLint. Frankly, I would prefer to trust the JSLint author. Douglas has a long history with Javascript and has written and lectured extensively on the language, and anyone who has been exposed to him knows that he understands JS inside out. He gets it.

I do not agree with everything that JSLint flags, I do think it is a fine aid for producing good, maintainable code and avoiding some hard-to-find bugs.

If you really don't like a particular flag, just change the JSLint source code (it's very readable and public domain) and run the code on your own server. Shouldn't take more than an afternoon to set it up.

Oh, and contrary to the author's previous comment. there is a server side implementation of JS - google node.js for details. Most people in JS community are well aware of this product.

Andrea Giammarchi said...

for standard you mean JSLint standard? This tool is not able to understand JS properly and sometimes it does not consider SPECS.

It's a good tool, but it does not ensure that much.

Finally, that HER is silly, there is even my picture beside ...

owca said...

https://github.com/douglascrockford/JSLint/issues/issue/20

Nathan said...

I don't agree with everything you've said, but good food for thought. I agree that certainly we as programmers should understand why we're coding in a certain style and not just following rules blindly.

(Regarding the "HER" in an earlier comment, I don't think that poster was trying to emphasise your sex, they were trying to emphasise that you are giving YOUR opinion and said "her" because in English "Andrea" is a girl's name. Your photo didn't load in my browser.)

Andrea Giammarchi said...

eheh Nathan, the point is that Andrew is the equivalent of my name and in Italy it is 99% masculine name so ... well, I like to underline it was a saint and it was a male, there is no equivalent saint for a woman and in any case ... well, I am not a saint, but I am bored to show my ID every time I pay with my card with my name :D

Anonymous said...

current jslint improves coding quality and performance, the author of this article has no idea of anything.

Andrea Giammarchi said...

sure ;)

Anonymous said...

Tabs rule, spaces are messy and difficult to maintain.

TeMc said...

Most of this post is gubbish and the tone of an inexperienced JavaScript programmer.

Just two random points:

== var statements at the top ==
This doesn't mean "var a = 1, b = 2;" instead of "var a = 1; var b = 2;" per se. The code conventions document recommends the variable be declared at the top, not defined. There is a big difference in JavaScript.

For example:
--code--
function bar() {
MYAPP.dialog( somethingUndeclared );
}
--/code--

Assuming MYAPP is a global object with a dialog method, and somethingUndeclared doesn't appear anywhere in the script. This will throw a ReferenceError exception because the variable is not declared anywhere and contrary to PHP this doesn't default to NULL or anything.

--code--
function bar() {
MYAPP.dialog( somethingUndeclared );

var somethingUndeclared = 5;

MYAPP.dialog( somethingUndeclared );
}
--/code--

In the above neither reference will throw an exception. WTF ? Guess what, as Mr D has warned us about, variables declarations are hoisted to the top. This is how the language works, deal with it. If you write the above, you don't know JavaScript.
One should write the way the language works that you're writing in. Not the way you like to make something look based on a completely different programming langauge. The below results in exactly the same and is basically what happeneds with the above at run time:


--code--
function bar() {
var somethingUndeclared;

MYAPP.dialog( somethingUndeclared );

somethingUndeclared = 5;

MYAPP.dialog( somethingUndeclared );
}
--/code--

Now, knowing variables in JavaScript default to type "undefined" when not assigned anything, and that String(undefined) (notice, not String("undefined"). This first alert will alert the word /string "undefined".

Same thing with the "i" variable from for loops. Since JavaScript doesn't warn when redeclaring an already declared variable, and since there is no block scope in JavaScript, it is important to know at all times that the "i" variable is visible in the entire function, including any of the potentially multiple and/or nested for-loops. If you start a function with a for loop, then about 30 lines and then another one, would you still remember that that i variable was already used ? If you learn to use one var statement per function and to put it at the top, then, when you'd go and add "i" for the loop you're about to write, you'd find out it is already in the var list of this function, and never risk re-using the same variable and running into what could be an infinite loop (as one loop could reset the other one since they share the same variable).

== function assignment (in)consistency ==
Lets look at the example again:

var collection = (function () {
var keys = [], values = [];

return {
....
};
}());

"collection" is NOT assigned a function, it's an object literal returned by an immediatly invoked (anonymous) function expression. This is one of the most powerful principles in all of JavaScript. The object literal continous to enjoy access to this function scope even after it is returned.

I'd heavily recommend watching two pieces of material:
* JavaScript: The Good Parts (you probably know this one already)
* Crockford on JavaScript (8-part series at YUI Theatre) - especially 3: Function the Ultimate.

Andrea Giammarchi said...

I stopped here:
Most of this post is gubbish and the tone of an inexperienced JavaScript programmer.

I write JS since 2000 so when you will be experienced enough to understand JS first, and this post after, feel free to come back and revisit everything you wrote in above comment.

Best Regards

TeMc said...

The last constructor example can be written in a better way which is imho better and passes JSLint accordingly.

Before that, below is the version as in the article:

--code--
"use strict";
var Singleton = new function () {
// private function (or method)
function _setValue(value) {
// lots of stuff here
_value = value;
}
var _value;
this.get = function () {
return _value;
};
this.set = function (value) {
if (!_value) {
_setValue(value);
}
};
this.constructor = Object;
};
--/code--

Singleton is not a constructor, that's one problem already. singleton is an object, not a constructor. If anything, that line should be:

var instance = new function Singleton() {

Anyway, here's the "alternative" way, which is sorter and faster (as it returns an object literal at once instead of using the ugly "this" and augmenting it.

var instance = (function () {
// local:
var foo;

function parseAndSetFoo(value) {
/* logic here */
foo = value;
}

// public:
return {
getFoo: function () {
return foo;
},
setFoo: function (value, force) {
if (foo === undefined || force) {
parseAndSetFoo(value);
}
}
};

}());

If you are worried about "undefined":
* Wrap all files in a closure. This is also a good measure to keep local scope and avoid leakage from and to other scopes. As well as speeds up frequently used globals that then become locally aliased:

(function ( $, M, undefined ) {
/* .... */

}( jQuery, MYAPP ));

Andrea Giammarchi said...

{}
that is singleton

guilllo said...

I'll be honest I haven't read the whole article. I stopped after the first 2/3 critics not because they are critics, but because it seems you just didn't get the point.

When it's said "The var statements should be the first statements in the function body." it doesn't mean that each declaration should be on its own line, it means that all declaration are declared at the beginning of a function. If you're coming from other languages you might assume that declaring a variable inside a "if" will take only space in memory if the... "if" is ran. It's not the case in Javascript, it will always be declared and take space, so it makes sense to put all the declaration at the same place and at the beginning of the function.

About the "Implied global variables should never be used." also, you didn't get it right. It means that if you want to use a global variable, declare it with the global keyword, not nothing. Maybe it was ok at the time Javascript came out, but it's not anymore (I think it throws an error now with ES5 if you use "use strict"). It does make sense also, and it shouldn't have been allowed at the beginning of javascript. The less worst would have been to set a variable as local if you don't use the var keyword, but absolutely not as a global. Would have probably avoid a lot of headaches for beginners.

Unknown said...

This blog post is considered harmful and demonstrates a lack of competence.

Andrea Giammarchi said...

very ... VERY constructive

waikit kung said...

Read JavaScript: The Good Parts again. Takes the things you agree and ignore the others.

Something is not absolute correctly for all scenarios:
e.g.
== and ===
usage of == may save some typing and maybe look clean to experienced JS programmers who know the table of type coercion rules, but I am not sure who will read and maintain my code in a large web project. I would usually choose to use === because I have to ensure that nobody will mis-understand it.
The little performance gain (reduce native scope chain lookup) is not my concern for nowadays fast and efficient JS interpreters. Maintenance is more important as long as the performance is acceptable for my web app target audience.

Andrea Giammarchi said...

although, if maintenance is done by people that don't know basics of JavaScript ... good luck with that!