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.
