Whether you use 2 spaces or 4 spaces, there are a few simple things that can make your node.js code easier to read. We've been using them in all the hapi modules for over 4 years now to great results. This list is by no means complete but it highlights the most useful elements that will give you immediate value in reducing bugs.
JavaScript makes it harder than most languages to know where variables are coming from. Variables assigned required modules are particularly important because they represent a singleton object shared with the entire application. There are also globals and module globals, along with function variables and arguments.
Traditionally, variables starting with an uppercase letter represent a class that must be instantiated using new. This was an important semantic in the early days of JavaScript but at this point, if you don't know Date requires new Date() you are probably very new. We have adopted Upper Camel Case variable names for all module global variables which are assigned required modules:
var Hapi = require('hapi');Note that you cannot new Hapi(), only new Hapi.Server(). In this style, the exported object from the modules always exposes an object which contains the API. This means a single function module should still export an object with the single method as an object property:
exports.add = function (a, b) {
return a + b;
};This makes it trivial to identify which variables in your code represent required modules. The language itself does contain a few uppercase variables but those are well known and should not cause any confusion.
When a module (any node.js file) contains its own prototype class, that class variable also starts with an uppercase which can cause confusion. However, we do not allow any module global variable except for one: internals (and of course whatever node.js provides).
var Hapi = require('hapi');
var internals = {};
internals.Server = function () {
this.server = new Hapi.Server();
this.server.connection();
};
internals.server = new internals.Server();
// This is not allowed:
var server = new internals.Server();This means that within any function in your code, uppercase variables are either required modules APIs or JavaScript natives. Any internals prefixed variables are module globals which act as a singleton. The rest are either function variables or arguments which are easier to spot because of the smaller scope.
A note about singletons - I often see bugs caused by developers using a module global variable used by a prototype. For example:
var internals = {};
internals.storage = {};
exports.Cache = internals.Cache = function () {
};
internals.Cache.prototype.get = function (key) {
return internals.storage[key];
};
internals.Cache.prototype.set = function (key, value) {
return internals.storage[key] = value;
};The problem is that multiple instances of the cache will all share the same memory:
var Cache = require('./cache');
var cache1 = new Cache();
var cache2 = new Cache(); // Uses the same memory cache1 usesBy requiring the use of internals as a prefix, this bug becomes obvious to spot. When I code review a pull request, I always search for all the instances of internals to make sure it only holds static configuration and other safe data to share between instances. This is much harder to spot without the prefix.
Understanding your code flow is critical in asynchronous development. Consider this:
internals.read = function (db, key, callback) {
db.get(key, function (err, value, meta) {
if (err) {
return callback(err);
}
if (meta.age > 10000) {
return callback(null, null);
}
return callback(null, value);
});
};At a quick glance, it is not obvious that the db.get() method is asynchronous. It looks just like any other scope indentation. By adding a new line after every function declaration, we make the callbacks "pop" and much easier to spot:
internals.read = function (db, key, callback) {
db.get(key, function (err, value, meta) {
if (err) {
return callback(err);
}
if (meta.age > 10000) {
return callback(null, null);
}
return callback(null, value);
});
};Allowing your eyes to immediately identify those line breaks means you can quickly note where your code switched event loop ticks.
You might have noticed that every callback() call is prefixed with return. This is a defensive programming tool. It provides both a visual cue where in the code execution ends and returned up the stack, but also protection against adding code later after that point that can create timing issues, race conditions, or multiple callback calls.
It is sometimes helpful to add a return when it is not obvious a nested function contains a callback. Consider the example above, it can be made safer with an extra return either in front of the method:
internals.read = function (db, key, callback) {
return db.get(key, function (err, value, meta) {
if (err) {
return callback(err);
}
if (meta.age > 10000) {
return callback(null, null);
}
return callback(null, value);
});
};or immediately after (usually with a comment):
internals.read = function (db, key, callback) {
db.get(key, function (err, value, meta) {
if (err) {
return callback(err);
}
if (meta.age > 10000) {
return callback(null, null);
}
return callback(null, value);
});
return; // db.get() invokes the callback
};There are many cases where a callback doesn't have to be called on the next tick for performance or workflow reasons. However, that can create an unpredictable API where it is not clear in which tick the callback is invoked. To make it explicit, I use callback when it is guaranteed the callback is called on another tick, and next when it can be both. It is a simple naming convention that has made maintaining hapi much simpler over many months.
exports.a = function (a, b, callback) {
return process.nextTick(function () {
return callback(null, a + b);
});
};
exports.b = function (a, b, next) {
return next(null, a + b);
};