Conventional JavaScript style has never sat well with me. I always had a feeling that something was “off” about the way we normally write JavaScript. I finally got to the bottom of it.
Here’s an idiomatic snippet of node.js code from the Express documentation:
app.get('/', function (req, res) {
res.send('GET request to homepage');
});
What could possibly be wrong with a three-line block of code?
The body of the function should be indented twice. Huh? While I don’t endorse throwing an extra two spaces in there and calling it a day, that is at the heart of the issue. The res.send belongs to the block of the inline function, and that function is a child/parameter to app.get. Each of those deserves an indentation.
We can further see that something is wrong because of the last line. We’ve all seen extreme examples where a big, ugly block of code terminates with a reassuring }]));. This is playing hot potato with whatever happened above it. It’s intellectually sloppy, it’s unhelpful to somebody reading the code, and it’s an indication that something is broken. Yes, your text editor can probably help you make sense of it. No, that’s not a good excuse. Yes, this saves us vertical space. No, that’s not a good tradeoff for clarity and readability.
You should consider writing it like this instead:
app.get(
'/',
function (req, res) {
res.send('GET request to homepage');
}
);
Had you noticed '/' in there before? It can easily be lost in the shuffle, but it’s just as important as its sibling, the inline function.
I understand that this code is considered uglier, but good code is written to be read and understood by humans. That should always trump aesthetics.
The same thing will happen when a function is being passed an array or object literal. For example,
something([
'abc',
'def'
]);
which is almost indistinguishable from
something(
'abc',
'def'
);
but means something very different (it may help to imagine that 'abc' and 'def' are long, so that you wouldn’t want to write this all on one line). The items in the array are indented wrong, as if they were children of something rather than grandchildren. As before, this can be rewritten
something(
[
'abc',
'def'
]
);
where it now becomes clear that an array is being passed to the function, and you don’t need to be careful to notice the [ and ] camouflaged on the first and last lines.
I get the feeling that some consider parentheses, brackets and curly braces as unnecessary baggage that should be stowed as well out of sight as possible. In a language that eschews them, maybe that case can be made. In JavaScript, that viewpoint is misguided: the symbols carry important meaning and context.
While I’ve said the problem is indentation, there is an almost equivalent formulation of the problem which I can’t articulate quite as well: passing “incomplete thought” parameters to functions. That is,
foo('abc', 'def', function() {
reads as
foo(<complete thought>, <complete thought>, <incomplete thought>
This is particularly bad when that incomplete thought is the prototype of a function, and the next line references a variable that only entered scope with the function… but you missed it because the function prototype was somewhere up-and-to-the-right. I have seen code like this, spilling off the screen to the right:
foo(........, ........, ........, ........, function(req, res) {
res.end();
});
Unless you scroll to the side and see that res has entered scope, you think it must be a global variable. You could help this by line length restrictions, but that treats the symptom rather than the problem.
Now consider the case of a function that accepts two functions as parameters (they make them like that??)
app.get('/', function (req, res, next) {
console.log('I'm a piece of middleware');
next();
}, function (req, res) {
res.send('GET request to homepage');
});
This code is incredibly lopsided. Here it is again, color-coded:
app.get('/', function (req, res, next) { console.log('I'm a piece of middleware'); next(); }, function (req, res) { res.send('GET request to homepage'); });
app.get but there’s no symmetry between them, each beginning at a wildly different position.}.app.get.It’s a mess. Rewriting as
app.get( '/', function (req, res, next) { console.log('I'm a piece of middleware'); next(); }, function (req, res) { res.send('GET request to homepage'); } );
solves all of those problems at the cost of 4 additional lines of code.
If your function call can fit comfortably on one line, great! Otherwise, I recommend putting each argument on its own line.
Good:
function handler(req, res) {
res.send('GET request to homepage');
}
app.get('/', handler);
Good:
app.get(
'/',
function (req, res) {
res.send('GET request to homepage');
}
);
Bad:
app.get('/', function (req, res) {
res.send('GET request to homepage');
});
Now that I’ve written this out, it seems like a trivial observation. Maybe everyone has already thought this through and decided to optimize for vertical compactness over clarity, but I’ve never seen this discussed in depth.
As I said at the start, conventional JavaScript style has always bothered me. While the standard style guides make all sorts of aesthetic choices that I don’t like (e.g. camelCase), I always knew that my aversion was rooted in something less superficial than “enclose string literals in apostrophes” (cough, “single quotes”). Writing this article helped me organize my thoughts and identify the real issue, which I consider one of logical coherence rather than aesthetics.
There’s a general sense that JavaScript and “{ at end of line” were made for eachother, and this confused inlining of functions and data structures helps to reinforce that. I’ll note that, as a corollary to this article, the language can be decoupled in a natural way from that style, and the decision about curly brace placement becomes merely an aesthetic one (modulo automatic semicolon insertion stupidity).