Balderdash Style Guide

Balderdash Style Guide.md

Balderdash Style Guide

The following is our development style guide. We put this together as a handy reference on how we write our code. We work on lots of projects, and using conventions offers advantages to everyone working with the code base. Writing new code is faster, maintaining old code is easier, and iterative feature development happens faster with consistent development styles used by everyone.

Even after agreeing on this guide, we've all been guilty of delighting in a bit of forbidden fruit. It's out of habit. It's not a big deal, let's just try to move towards standards as quickly as we can. In the mean time, if we see code that doesn't match up, and we have time, let's fix it.

As usual, feel free to pipe up with ideas and errata. That said, let's avoid religious wars and subjective rules-- everything below is here for a reason, and we want everybody on board with the why. Following rules without knowing why they exist is stupid. The goal is to make our code cleaner, our development faster, and the whole process more fun.

Why Javascript?

Like any modern web studio, we're working mainly with Javascript, so is important that we start by standardizing conventions and the basics of the language.

Theory

Here are a few general principles we like to follow. Like any good theory, it contradicts itself sometimes, but only where practice has necessitated it.

• Always do things the same way-- it makes you nimble and wise
• Don't restate the obvious
• If you're frustrated that a convention doesn't exist, make one up and tell everybody about it
• It's ok to do crazy things sometimes, but when you do, document the hell out of it!
• But don't be too tricky-- write code you would be able to understand 6 months ago.

Semicolons

Always. Too risky not to. If you don't want to use semicolons, write CoffeeScript. We're ok with that.

if / else

Always use braces ({}) in your if/else statements, even if they're only one line.

Exceptions:

• Extremely short truth test where the if and the then statement can be written on one line:

if (err) return res.json;

• Same thing for else:

if (foo) {
	// do stuff
}
else bar();

Ternary operators

These are a great way to keep code concise. Some of us (ahem Mike) are particularly fond of putting together horrifying, disgusting ternary operator chains that take minutes to unravel. This is a bad idea. I've learned my lesson.

Honestly, most of the time it's better just to use an if statement.

E.g. bad

var username = email.length < 50 ? email : fullname.length < 50 ? fullname : id;

Good

var username;
if (email.length < 50) {
	username = email;
}
else if (fullname.length < 50) {
	username = fullname;
}
else username = id;

Good sometimes

// This is handy because we can put everything on one line
// But not a good idea for the example above (note that I excluded the fullname stuff)
var username = (email.length < 50) ? email : id;

Boolean short circuits

These are awesome. The biggest thing to remember is not to get carried away and do things that would be much easier just to write with a simple one-line if statement.

An example of good short-circuits:

// OR short-circuit
// If it was specified, use the username request parameter, otherwise use email
var username = req.param('username') || req.param('email'); 

// AND short-circuit
// Grab the username property from the user object.  If the user object doesn't exist, return a falsy value.
// (this avoids throwing any null pointer exceptions)
var username = user && user.username;

for and while loops

Use underscore. For and while loops are crazy in javascript.

switch statements

If you have > 2 disjoint cases you're checking against (i.e. the thing you're checking matches specifically ONE of them), then a switch statement is a good idea. Otherwise, it's probably not. Again, not because of efficiency, or correctness, or anything like that-- this is all about making code easier to read for each other.

Flow control

For dealing with multiple asynchronous calls that need to run in series and parallel, use async. https://github.com/caolan/async/

Asynchronous functions

The last argument of any asynchronous function should be a callback. How do you know a function is asynchronous? It won't have an answer to respond with until one of its callbacks fires.

As an alternative notation, for very commonly used functions, you'll sometimes see the use of a promise. Promises can help make it clear what additional arguments are doing. E.g.

User.findAll().limit(10).done(function (err, users) {
	// ...
})

However, it's not usually worth the time to write application-level functions like this-- only if you're working on a library or tool.

Callback functions

• Always use return when you're triggering a callback function. Even though you're not planning on returning anything, using return prevents unintended consequences and implies that you mean for this to be an exit point from the asynchronous flow. E.g.

function (cb) {
	if (err) return cb(err);
	
	// do more stuff without worrying about wrapping it in an else
}

• The first argument of a callback function should always be an error. In your callback, before doing anything else, deal with the error. E.g.

function (req,res) {
	User.find(7428).done(function (err,result) {
		if (err) res.send(err,500);
		
		// ...
	});
}

Error handling

On the client-side, errors oftentimes halt all of the page's javascript, bringing the experience to a screeching halt. On the back-end, errors just flat out crash the server. This is some pretty heavy stuff.

When dealing with errors, we subscribe to the following tenants:

• In general, when you see unexpected input in a function with a callback, trigger the callback with a simple string error.
• If you see unexpected input from an end user (e.g. the DOM or a server request), trigger the callback with an error object. If you're in a piece of express middleware, res.send a simple string message (good) or res.json an error object (awesome) Just be consistent!
• If it's the kind of error you might want to relect in the UI someday, it's best to send back a JSON error so you have the flexibiliy of adding more data later on.

Code Comments

File comments

We use block commenting at the beginning of a file to give a short description of what that file is for. No need to go into depth, but especially when the file is non-standard (i.e. not a controller, model, component, or view) it helps to give a succinct explanation of the file's purpose.

If you're using sails generate, a comment will be injected for you. No need to write more, since the type of file it is should speak for itelf.

e.g.

/*
* FruitController.js
*/

Method/Function comments

We sometimes use inline comments to provide an abbreviated description of what a method or function does. If the purpose is self-evident, it's not helpful to restate that-- instead, comments should help explain the arguments and return value of the function. If the function uses a callback, the comment might explain the arguments the callback accepts. If the function uses a return value and a function (wtf) hopefully you have a reason, so explain yourself.

e.g.

Not so helpful

// Looks up a file by path
function lookupFileByPath (path, cb) {
41// ...
}

Better

This is fine since the function has an expressive name and we have callback conventions

function lookupFileByPathname(path, cb) {
	
}

Best

This comment doesn't restate the function's purpose, but provides a piece of useful information that may or may not be clear: the arguments to the callback. Even though we have standards around this sort of a callback, it doesn't hurt. Realistically, we would never be writing a function this simple since most of this sort of thing is baked into Waterline.

// cb	: (err, file)
function lookupFileByPath (path, cb) {
	// ...
}

Intra-function comments

The same principle applies for comments inside of functions. We've opted out of off-to-the-right comments (too hard to see when your editor is half-screened), and instead opted for the more spaced out above-the-line comments. We also like including a newline before comments.

Example

// Inject the phone number into the DOM 
// after stripping non-numeric characters
if (phoneNumber && _.isString(phoneNumber)) {
	phoneNumber = phoneNumber.replace(/[^0-9]/g,'');
	$('span.phone-number').text(phoneNumber);
}

Nonexample #1 Too hard to read in a narrow window, and easy to get malformated

if (phoneNumber) { // Inject a phone number into the DOM, stripping non-numeric characters
	phoneNumber = phoneNumber.replace(/[^0-9]/g,'');
	$('span.phone-number').text(phoneNumber); // insert the phone number into span element
}

Nonexample #2 Omitting the line before the comment can get crowded and hard to read

// Check to see if the phone number truthy.
if (phoneNumber) {
	// Strip non-numeric characters
	phoneNumber = phoneNumber.replace(/[^0-9]/g,'');
	// insert the phone number into span element
	$('span.phone-number').text(phoneNumber);
}