How to write Javascript

…when you're not drunk

You probably already think you know how to write Javascript when you're not drunk. But I hear you hating.

It's ok to have written some bad Javascript. No one writes the pure, innocent Javascript of Eden. We are all flawed.

When you wake up and realize what's happened, your friends are the ones who help you take care of it.

You started with some simple pseudocode…

apiKey = loadConfig();
userId = authenticateUser(apiKey);
client = new ApiClient(userId);
info = client.getUserInfo();
writeResponse(client.format, serialize(info));

You wrote some async Javascript…

var apiKey = fs.readFileSync(SETTINGS).apiKey;
var client = new ApiClient();
var info;
async.waterfall([
	function(callback) {
		authenticateUser(apiKey, callback);
	},
	function(callback) {
		client.getUserInfo(function(err, result) {
			info = result.info;
			callback();
		});
	},
	function(callback) {
		writeResponse(client.format, serialize(info), callback);
	}
], callback);

You couldn't construct `ApiClient` until you had a `userId`, ok so move that around

var apiKey = fs.readFileSync(SETTINGS).apiKey;
var client, info;
async.waterfall([
	function(callback) {
		authenticateUser(apiKey, callback);
	},
	function(result, callback) {
		// can't create the client until we have the userId
		client = new ApiClient(result.userId);
		client.getUserInfo(function(err, result) {
			info = result.info;
			callback();
		});
	},
	function(callback) {
		writeResponse(client.format, serialize(info), callback);
	}
], callback);

You needed some conditionals, just add that in there…


	function(callback) {
		if (!userId) {
			info = defaultInfo;
			callback();
			return;
		}
		// can't create the client until we have the userId
		client = new ApiClient(userId);
		client.getUserInfo(function(err, result) {
			info = result.info;
			callback();
		});
	},

Then you had to adjust for the argument order of someone else's method…


	function(callback) {
		writeResponse(client.format, serialize(info), callback);
	},
	function(result, callback) {
		reportMetrics(result.latency, result.statusCode);
	}

Let's review

Pseudocode

apiKey = loadConfig();
userId = authenticateUser(apiKey);
client = new ApiClient(userId);
info = client.getUserInfo();
writeResponse(client.format, serialize(info));

Reality

var apiKey = fs.readFileSync(SETTINGS).apiKey;
var client, info;
async.waterfall([
	function(callback) {
		authenticateUser(apiKey, callback);
	},
	function(callback) {
		if (!userId) {
			info = defaultInfo;
			callback();
			return;
		}
		// can't create the client until we have the userId
		client = new ApiClient(userId);
		client.getUserInfo(function(err, result) {
			info = result.info;
			callback();
		});
	},
	function(callback) {
		writeResponse(client.format, serialize(info), callback);
	},
	function(result, callback) {
		reportMetrics(result.latency, result.statusCode);
	}
], callback);

Imagine the preceding example repeated hundreds of times within 200,000 lines of code …

Tiny factoring impedance adds up

You can reach these problems iteratively

Very simple factoring works fine for smaller projects and modules

It can even be elegant.

There's a cliff you hit in complexity, sometimes very late at night… and you might just add one commit too many and lose control.

Don't be the one who takes things too far.

It's ok don't worry chill you can fix it

After some time has passed, it is time to revisit your factoring

Bugs hide in lines of code
Don't overdo it
- The readability of inline anonymous functions is one of Javascript's virtues
Sacrifice less for flow control
Nested scopes are untestable and share too much state
Untestable shared state breaks simple changes
Truly anonymous functions make terrible stack traces

Having to change the interface of a class or function is the worst.

How to ZOOM through huge refactors by rewriting the `arguments` array:

Yeah, I was kidding. Don't do that.
Don't monkey-patch objects or modules either
There is so much bad code to learn from
Feeling the urge to do something crazy means it is time to refactor to remove the temptation
When in doubt, listen to your heart

What does it take to get async code that looks like this?

async.waterfall([
	authenticateUser,
	client.getUserInfo,
	writeResponse
], callback);

You can't.

`this` is too unwieldy
You use third-party libraries with varying parameter order
Even if it works today, a requirement change will break this tight coupling tomorrow
You just aren't that lucky

Instead, use a class

Just a thought. An async workflow is a state machine. A class is a collection of state and operations upon that state. Consider just writing a class for each workflow, exposing each step as a method, and keeping all shared state in the class instead of embedded in lexical scope.

The methods will be individually testable, tests can create fixture instances of the class, and you can use dependency injection within the class to mock external resources.

Assume the ideal method signatures and return values, write the async workflow first, and write your class later

Just one possible way to go:

async.waterfall([
	this.auth.bind(this),
	this.getUserInfo.bind(this),
	this.writeResponse.bind(this)
], callback);

(too bad about having to bind `this`)

Oh, and never use `async.waterfall`, it tightly couples callback parameters to method signatures. Use `async.series` or `async.auto`.

async.series([
	this.auth.bind(this),
	this.getUserInfo.bind(this),
	this.writeResponse.bind(this)
], callback);

Each method can encapsulate the weirdness required for the code it interacts with. You could even try using polymorphism for type dispatch instead of conditional statements inside them.

You may also want to refactor by removing a class or two

Inline anonymous functions sometimes increase clarity

Javascript isn't Java or Ruby— seek the most consistent idiomatic expression

Sometimes you can remove abstractions and layers of callbacks to find some very simple code