- Introduction
- Variables
- Functions
- Objects and Data Structures
- Classes
- Testing
- Concurrency
- Error Handling
- Formatting
- Comments
Software engineering principles, from Robert C. Martin's book Clean Code, adapted for JavaScript. This is not a style guide. It's a guide to producing readable, reusable, and refactorable software in JavaScript.
Not every principle herein has to be strictly followed, and even fewer will be universally agreed upon. These are guidelines and nothing more, but they are ones codified over many years of collective experience by the authors of Clean Code.
Our craft of software engineering is just a bit over 50 years old, and we are still learning a lot. When software architecture is as old as architecture itself, maybe then we will have harder rules to follow. For now, let these guidelines serve as a touchstone by which to assess the quality of the JavaScript code that you and your team produce.
One more thing: knowing these won't immediately make you a better software developer, and working with them for many years doesn't mean you won't make mistakes. Every piece of code starts as a first draft, like wet clay getting shaped into its final form. Finally, we chisel away the imperfections when we review it with our peers. Don't beat yourself up for first drafts that need improvement. Beat up the code instead!
Bad:
const yyyymmdstr = moment().format('YYYY/MM/DD');Good:
const yearMonthDay = moment().format('YYYY/MM/DD');Bad:
getUserInfo();
getClientData();
getCustomerRecord();Good:
getUser();We will read more code than we will ever write. It's important that the code we do write is readable and searchable. By not naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like buddy.js and ESLint can help identify unnamed constants.
Bad:
// What the heck is 86400 for?
setTimeout(() => {
this.blastOff()
}, 86400);Good:
// Declare them as capitalized `const` globals.
const SECONDS_IN_A_DAY = 86400;
setTimeout(() => {
this.blastOff()
}, SECONDS_IN_A_DAY);Bad:
const address = 'One Infinite Loop, Cupertino 95014';
const cityStateRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
saveCityState(address.match(cityStateRegex)[1], address.match(cityStateRegex)[2]);Good:
const address = 'One Infinite Loop, Cupertino 95014';
const cityStateRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
const [, city, state] = address.match(cityStateRegex);
saveCityState(city, state);Explicit is better than implicit.
Bad:
const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((l) => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
// Wait, what is `l` for again?
dispatch(l);
});Good:
const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((location) => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
dispatch(location);
});If your class/object name tells you something, don't repeat that in your variable name.
Bad:
const Car = {
carMake: 'Honda',
carModel: 'Accord',
carColor: 'Blue'
};
function paintCar(car) {
car.carColor = 'Red';
}Good:
const Car = {
make: 'Honda',
model: 'Accord',
color: 'Blue'
};
function paintCar(car) {
car.color = 'Red';
}Bad:
function createMicrobrewery(name) {
const breweryName = name || 'Hipster Brew Co.';
...
}Good:
function createMicrobrewery(breweryName = 'Hipster Brew Co.') {
...
}Limiting the amount of function parameters is incredibly important because it makes testing your function easier. Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument.
Zero arguments is the ideal case. One or two arguments is ok, and three should be avoided. Anything more than that should be consolidated. Usually, if you have more than two arguments then your function is trying to do too much. In cases where it's not, most of the time a higher-level object will suffice as an argument.
Since JavaScript allows us to make objects on the fly, without a lot of class boilerplate, you can use an object if you are finding yourself needing a lot of arguments.
Bad:
function createMenu(title, body, buttonText, cancellable) {
// ...
}Good:
const menuConfig = {
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
};
function createMenu(config) {
// ...
}This is by far the most important rule in software engineering. When functions do more than one thing, they are harder to compose, test, and reason about. When you can isolate a function to just one action, they can be refactored easily and your code will read much cleaner. If you take nothing else away from this guide other than this, you'll be ahead of many developers.
Bad:
function emailClients(clients) {
clients.forEach((client) => {
const clientRecord = database.lookup(client);
if (clientRecord.isActive()) {
email(client);
}
});
}Good:
function emailClients(clients) {
clients
.filter(isClientActive)
.forEach(email);
}
function isClientActive(client) {
const clientRecord = database.lookup(client);
return clientRecord.isActive();
}Bad:
function addToDate(date, month) {
// ...
}
const date = new Date();
// It's hard to to tell from the function name what is added
addToDate(date, 1);Good:
function addMonthToDate(month, date) {
// ...
}
const date = new Date();
addMonthToDate(1, date);When you have more than one level of abstraction your function is usually doing too much. Splitting up functions leads to reusability and easier testing.
Bad:
function parseBetterJSAlternative(code) {
const REGEXES = [
// ...
];
const statements = code.split(' ');
const tokens = [];
REGEXES.forEach((REGEX) => {
statements.forEach((statement) => {
// ...
});
});
const ast = [];
tokens.forEach((token) => {
// lex...
});
ast.forEach((node) => {
// parse...
});
}Good:
function tokenize(code) {
const REGEXES = [
// ...
];
const statements = code.split(' ');
const tokens = [];
REGEXES.forEach((REGEX) => {
statements.forEach((statement) => {
tokens.push( /* ... */ );
});
});
return tokens;
}
function lexer(tokens) {
const ast = [];
tokens.forEach((token) => {
ast.push( /* ... */ );
});
return ast;
}
function parseBetterJSAlternative(code) {
const tokens = tokenize(code);
const ast = lexer(tokens);
ast.forEach((node) => {
// parse...
});
}Never ever, ever, under any circumstance, have duplicate code. There's no reason for it and it's quite possibly the worst sin you can commit as a professional developer. Duplicate code means there's more than one place to alter something if you need to change some logic. JavaScript is untyped, so it makes having generic functions quite easy. Take advantage of that! Tools like jsinspect can help you find duplicate code eligible for refactoring.
Bad:
function showDeveloperList(developers) {
developers.forEach((developer) => {
const expectedSalary = developer.calculateExpectedSalary();
const experience = developer.getExperience();
const githubLink = developer.getGithubLink();
const data = {
expectedSalary: expectedSalary,
experience: experience,
githubLink: githubLink
};
render(data);
});
}
function showManagerList(managers) {
managers.forEach((manager) => {
const expectedSalary = manager.calculateExpectedSalary();
const experience = manager.getExperience();
const portfolio = manager.getMBAProjects();
const data = {
expectedSalary: expectedSalary,
experience: experience,
portfolio: portfolio
};
render(data);
});
}Good:
function showList(employees) {
employees.forEach((employee) => {
const expectedSalary = employee.calculateExpectedSalary();
const experience = employee.getExperience();
let portfolio = employee.getGithubLink();
if (employee.type === 'manager') {
portfolio = employee.getMBAProjects();
}
const data = {
expectedSalary: expectedSalary,
experience: experience,
portfolio: portfolio
};
render(data);
});
}Bad:
const menuConfig = {
title: null,
body: 'Bar',
buttonText: null,
cancellable: true
};
function createMenu(config) {
config.title = config.title || 'Foo';
config.body = config.body || 'Bar';
config.buttonText = config.buttonText || 'Baz';
config.cancellable = config.cancellable === undefined ? config.cancellable : true;
}
createMenu(menuConfig);Good:
const menuConfig = {
title: 'Order',
// User did not include 'body' key
buttonText: 'Send',
cancellable: true
};
function createMenu(config) {
config = Object.assign({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
}, config);
// config now equals: {title: "Order", body: "Bar", buttonText: "Send", cancellable: true}
// ...
}
createMenu(menuConfig);Flags tell your user that this function does more than one thing. Functions should do one thing. Split out your functions if they are following different code paths based on a boolean.
Bad:
function createFile(name, temp) {
if (temp) {
fs.create(`./temp/${name}`);
} else {
fs.create(name);
}
}Good:
function createFile(name) {
fs.create(name);
}
function createTempFile(name) {
createFile(`./temp/${name}`);
}A function produces a side effect if it does anything other than take a value in and return another value or values. A side effect could be writing to a file, modifying some global variable, or accidentally wiring all your money to a stranger.
Now, you do need to have side effects in a program on occasion. Like the previous example, you might need to write to a file. What you want to do is to centralize where you are doing this. Don't have several functions and classes that write to a particular file. Have one service that does it. One and only one.
The main point is to avoid common pitfalls like sharing state between objects without any structure, using mutable data types that can be written to by anything, and not centralizing where your side effects occur. If you can do this, you will be happier than the vast majority of other programmers.
Bad:
// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
let name = 'Ryan McDermott';
function splitIntoFirstAndLastName() {
name = name.split(' ');
}
splitIntoFirstAndLastName();
console.log(name); // ['Ryan', 'McDermott'];Good:
function splitIntoFirstAndLastName(name) {
return name.split(' ');
}
const name = 'Ryan McDermott';
const newName = splitIntoFirstAndLastName(name);
console.log(name); // 'Ryan McDermott';
console.log(newName); // ['Ryan', 'McDermott'];Polluting globals is a bad practice in JavaScript because you could clash with another
library and the user of your API would be none-the-wiser until they get an
exception in production. Let's think about an example: what if you wanted to
extend JavaScript's native Array method to have a diff method that could
show the difference between two arrays? You could write your new function
to the Array.prototype, but it could clash with another library that tried
to do the same thing. What if that other library was just using diff to find
the difference between the first and last elements of an array? This is why it
would be much better to just use ES2015/ES6 classes and simply extend the Array global.
Bad:
< F438 div class="highlight highlight-source-js notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="Array.prototype.diff = function diff(comparisonArray) { const values = []; const hash = {}; for (const i of comparisonArray) { hash[i] = true; } for (const i of this) { if (!hash[i]) { values.push(i); } } return values; };">Array.prototype.diff = function diff(comparisonArray) { const values = []; const hash = {}; for (const i of comparisonArray) { hash[i] = true; } for (const i of this) { if (!hash[i]) { values.push(i); } } return values; };