👎Bad:
const yyyymmdstr = moment().format('YYYY/MM/DD');
👍Good:
const currentDate = moment().format('YYYY/MM/DD');
👎Bad:
getUserInfo();
getClientData();
getCustomerRecord();
👍Good:
getUser();
👎Bad:
// What the heck is 86400000 for?
setTimeout(blastOff, 86400000);
👍Good:
// Declare them as capitalized named constants.
const MILLISECONDS_IN_A_DAY = 86400000;
setTimeout(blastOff, MILLISECONDS_IN_A_DAY);
👎Bad:
const address = 'One Infinite Loop, Cupertino 95014';
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
saveCityZipCode(address.match(cityZipCodeRegex)[1], address.match(cityZipCodeRegex)[2]);
👍Good:
const address = 'One Infinite Loop, Cupertino 95014';
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
const [_, city, zipCode] = address.match(cityZipCodeRegex) || [];
saveCityZipCode(city, zipCode);
👎Bad:
const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(l => {
doSomething(l);
});
👍Good:
const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach(location => {
doSomething(location);
});
👎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:
const createFullName = (firstName, lastName) => {
const fName = firstName || "Yauhen";
// ...
}
👍Good:
const createFullName = (firstName = "Yauhen", lastName = "Kavalchuk") => {
// ...
}
👎Bad:
const emailClients = (clients) => {
clients.forEach(client => {
const clientRecord = database.lookup(client);
if (clientRecord.isActive()) {
email(client);
}
});
}
👍Good:
const emailActiveClients = (clients) => {
clients.filter(isActiveClient).forEach(email);
}
const isActiveClient = (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);
👎Bad:
const createFile = (name, temp) => {
temp ? fs.create(`./temp/${name}`) : fs.create(name);
}
👍Good:
const createFile = (name) => {
fs.create(name);
}
const createTempFile = (name) => {
createFile(`./temp/${name}`);
}
👎Bad:
if (fsm.state === "fetching" && isEmpty(listNode)) {
// ...
}
👍Good:
function shouldShowSpinner(fsm, listNode) {
return fsm.state === "fetching" && isEmpty(listNode);
}
if (shouldShowSpinner(fsmInstance, listNodeInstance)) {
// ...
}
👎Bad:
function isDOMNodeNotPresent(node) {
// ...
}
if (!isDOMNodeNotPresent(node)) {
// ...
}
👍Good:
function isDOMNodePresent(node) {
// ...
}
if (isDOMNodePresent(node)) {
// ...
}
👎Bad:
function createMenu(title, body, buttonText, cancellable) {
// ...
}
👍Good:
function createMenu({ title, body, buttonText, cancellable }) {
// ...
}
createMenu({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
});
👎Bad:
Array.prototype.diff = function diff(comparisonArray) {
const hash = new Set(comparisonArray);
return this.filter(elem => !hash.has(elem));
};
👍Good:
class SuperArray extends Array {
diff(comparisonArray) {
const hash = new Set(comparisonArray);
return this.filter(elem => !hash.has(elem));
}
}
👎Bad:
// Bad
function oldRequestModule(url) {
// ...
}
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker('apples', req, 'test-url');
👍Good:
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker('apples', req, 'test-url');
👎Bad:
const Employee = function(name) {
this.name = name;
};
Employee.prototype.getName = function getName() {
return this.name;
};
const employee = new Employee('John Doe');
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
delete employee.name;
console.log(`Employee name: ${employee.getName()}`); // Employee name: undefined
👍Good:
const Employee = function (name) {
this.getName = function getName() {
return name;
};
};
const employee = new Employee('John Doe');
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
delete employee.name;
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
👎Bad:
class BankAccount {
constructor() {
this.balance = 1000;
}
}
const bankAccount = new BankAccount();
bankAccount.balance -= 100;
👍Good:
class BankAccount {
constructor(balance = 1000) {
this._balance = balance;
}
set balance(amount) {
if (this.verifyIfAmountCanBeSetted(amount)) {
this._balance = amount;
}
}
get balance() {
return this._balance;
}
verifyIfAmountCanBeSetted(val) {
// ...
}
}
const bankAccount = new BankAccount();
bankAccount.balance -= shoesPrice;
let balance = bankAccount.balance;
- Принцип единственной ответственности
- Принцип открытости/закрытости
- Принцип подстановки Барбары Лисков
- Принцип разделения интерфейса
- И принцип инверсии зависимостей
👎Bad:
class CarBuilder {
constructor() {
this.car = new Car();
}
addAutoPilot(autoPilot) {
this.car.autoPilot = autoPilot;
}
addParktronic(parktronic) {
this.car.parktronic = parktronic;
}
addSignaling(signaling) {
this.car.signaling = signaling;
}
updateEngine(engine) {
this.car.engine = engine;
}
build() {
return this.car;
}
}
const car = new CarBuilder();
car.addAutoPilot(true)
car.addParktronic(true)
car.updateEngine('V8')
car.build();
👍Good:
class CarBuilder {
constructor() {
this.car = new Car();
}
addAutoPilot(autoPilot) {
this.car.autoPilot = autoPilot;
return this;
}
addParktronic(parktronic) {
this.car.parktronic = parktronic;
return this;
}
addSignaling(signaling) {
this.car.signaling = signaling;
return this;
}
updateEngine(engine) {
this.car.engine = engine;
return this;
}
build() {
return this.car;
}
}
const car = new Car()
.addAutoPilot(true)
.addParktronic(true)
.updateEngine('V8')
.build();
👎Bad:
const assert = require('assert');
describe('MakeMomentJSGreatAgain', () => {
it('handles date boundaries', () => {
let date;
date = new MakeMomentJSGreatAgain('1/1/2015');
date.addDays(30);
date.shouldEqual('1/31/2015');
date = new MakeMomentJSGreatAgain('2/1/2016');
date.addDays(28);
assert.equal('02/29/2016', date);
date = new MakeMomentJSGreatAgain('2/1/2015');
date.addDays(28);
assert.equal('03/01/2015', date);
});
});
👍Good:
const assert = require('assert');
describe('MakeMomentJSGreatAgain', () => {
it('handles 30-day months', () => {
const date = new MakeMomentJSGreatAgain('1/1/2015');
date.addDays(30);
date.shouldEqual('1/31/2015');
});
it('handles leap year', () => {
const date = new MakeMomentJSGreatAgain('2/1/2016');
date.addDays(28);
assert.equal('02/29/2016', date);
});
it('handles non-leap year', () => {
const date = new MakeMomentJSGreatAgain('2/1/2015');
date.addDays(28);
assert.equal('03/01/2015', date);
});
});
👎Bad:
request.get(url, (requestErr, response) => {
requestErr ?
console.error(requestErr) :
fs.writeFile('article.html', response.body, (writeErr) => {
writeErr ? console.error(writeErr): console.log('File written');
});
});
👍Good:
requestPromise.get(url)
.then((response) => fsPromise.writeFile('article.html', response))
.then(() => { console.log('File written'); })
.catch((err) => { console.error(err); });
👎Bad:
requestPromise.get(url)
.then((response) => fsPromise.writeFile('article.html', response))
.then(() => { console.log('File written'); })
.catch((err) => { console.error(err); });
👍Good:
async function getCleanCodeArticle(url) {
try {
const response = await requestPromise.get(url);
await fsPromise.writeFile('article.html', response);
console.log('File written');
} catch(err) {
console.error(err);
}
}
👎Bad:
try {
functionThatMightThrow();
} catch (error) {
console.log(error);
}
👍Good:
try {
functionThatMightThrow();
} catch (error) {
console.error(error);
notifyUserOfError(error);
reportErrorToService(error);
}
👎Bad:
const DAYS_IN_WEEK = 7;
const daysInMonth = 30;
const songs = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const Artists = ['ACDC', 'Led Zeppelin', 'The Beatles'];
function eraseDatabase() {}
function restore_database() {}
class animal {}
class Alpaca {}
👍Good:
const DAYS_IN_WEEK = 7;
const DAYS_IN_MONTH = 30;
const songs = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const artists = ['ACDC', 'Led Zeppelin', 'The Beatles'];
function eraseDatabase() {}
function restoreDatabase() {}
class Animal {}
class Alpaca {}
👎Bad:
function hashIt(data) {
// The hash
let hash = 0;
// Length of string
const length = data.length;
// Loop through every character in data
for (let i = 0; i < length; i++) {
// Get character code.
const char = data.charCodeAt(i);
// Make the hash
hash = ((hash << 5) - hash) + char;
// Convert to 32-bit integer
hash &= hash;
}
}
👍Good:
function hashIt(data) {
let hash = 0;
const length = data.length;
for (let i = 0; i < length; i++) {
const char = data.charCodeAt(i);
hash = ((hash << 5) - hash) + char;
// Convert to 32-bit integer
hash &= hash;
}
}
👎Bad:
doStuff();
// doOtherStuff();
// doSomeMoreStuff();
// doSoMuchStuff();
👍Good:
doStuff();
👎Bad:
/**
* 2016-12-20: Removed monads, didn't understand them (RM)
* 2016-10-01: Improved using special monads (JP)
* 2016-02-03: Removed type-checking (LI)
* 2015-03-14: Added combine with type-checking (JR)
*/
function combine(a, b) {
return a + b;
}
👍Good:
function combine(a, b) {
return a + b;
}
👎Bad:
////////////////////////////////////////////////////////////////////////////////
// Scope Model Instantiation
////////////////////////////////////////////////////////////////////////////////
$scope.model = {
menu: 'foo',
nav: 'bar'
};
////////////////////////////////////////////////////////////////////////////////
// Action setup
////////////////////////////////////////////////////////////////////////////////
const actions = function() {
// ...
};
👍Good:
$scope.model = {
menu: 'foo',
nav: 'bar'
};
const actions = function() {
// ...
};