このリポジトリは個人資料の為、原文が必要な方は下記のリンクを参考にしてください。
clean code JavaScript(本家)
https://github.com/mitsuruog/clean-code-javascript
clean code JavaScript(日本語訳)
https://mitsuruog.github.io/clean-code-javascript/
https://github.com/misak1/clean-code-javascript
キレイなコードは3R(Readable、Reusable、Refactorable)になっていることを指し 読みやすい、再利用可能、理解や修正がしやすいように整理されている これらを目指すためのガイドになります。
全てのコードは、粘土が徐々に形作られていくように、曖昧な形から始まるものです。 その後でコードレビューを行い、不要なコードを削除して作りあげていくものです。 とも語られているそうです。
JavaScriptは良くも悪くも、非常に自由に書ける言語です。 その為、個人の技量に応じてコードが複雑になったりしやすいので、 改めてみてみると面白い発見がありましたので個人の見解も含めて紹介したいと思います。
・サンプルコードについて一部わかりにくいものについては差し替えました。
・業務で使うのがES6ということで、ES5関連の過去の記述方法については削除
・ES8や策定中の機能については削除
・デザインパターンについては話がややこしくなるので省略します。
利用範囲の広い変数には意味のある名前を利用しましょう 例にconst（定数）として使う場合にこれが該当します。
Bad:
const yyyymmdstr = getDate().format('YYYY/MM/DD');Good:
const currentDate = getDate().format('YYYY/MM/DD');Bad:
class User(){
getUserInfo();
}
class Client(){
getClientData();
}
class Customer(){
getCustomerRecord();
}Good:
class User(){
getUser();
}
class Client(){
getUser();
}
class Customer(){
getUser();
}マジックナンバーは他の人でも理解出来るように一度変数に代入しましょう。
Bad:
setTimeout(blastOff, 86400000);Good:
const MILLISECONDS_IN_A_DAY = 86400000;
setTimeout(blastOff, MILLISECONDS_IN_A_DAY);説明的な変数を利用しましょう
Bad:
let revision = '20171218164030';
let f = revision.match(/^(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/);
console.info(new Date(f[1], f[2], f[3], f[4], f[5], f[6])); // Thu Jan 18 2018 16:40:30 GMT+0900 (JST)Good:
let revision = '20171218164030';
let [, year, month, day, hour, minute, second] = revision.match(/^(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/);
console.info(new Date(year, month, day, hour, minute, second)); // Thu Jan 18 2018 16:40:30 GMT+0900 (JST)
match戻り値[0]を切り捨てる為に0番目を宣言しないことで捨てている(後述の分割代入構文)
明らかなことは暗黙的なことよりも優れています。 訳注：メンタルマップとは、認知心理学において記憶の中に構成される「あるべき姿」のイメージをさす言葉です。
Bad:
const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((l) => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
// Wait, what is `l` for again?
// ちょっと待って、もう一度`l`ってなんだっけ？
dispatch(l);
});Good:
const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((___location) => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
dispatch(___location);
}); 要するに他人に見せてもしょうがないコメントは不安を煽るだけだから残すなということ!?
forEachでループするものが___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';
}デフォルト引数は多くの場合、短絡評価よりも明確です。 ご存知の通り、これらを使った場合、関数は
undefinedの引数のみにデフォルト値を提供します。 他の''、""、false、null、0やNaNのような"falsy"値は、デフォルト値で置き換わることはありません。
Bad:
function createMicrobrewery(name) {
const breweryName = name || 'Hipster Brew Co.';
// ...
}Good:
function createMicrobrewery(breweryName = 'Hipster Brew Co.') {
// ...
} ES6になってからデフォルト引数が使えるようになっているので使っていきましょう。
ただ、IEとかモバイルサファリで使えない場合があるので、生のjsを扱う場合は注意です。
関数の引数の数を制限することは、テストを簡単に行えるという点において非常に重要なことです。 3つ以上あるということは、それぞれ別の引数を伴った数多くの異なるケースをテストしなければならないという問題につながります。
1つか2つの場合は理想的で、3つ以上になる場合は次の手法を検討してください。 ほとんどの場合、上位のオブジェクトを引数とすれば十分でしょう。
JavaScriptは、多くのクラスの雛形がなくとも素早くオブジェクトを作成することができるため、もし多くの引数を必要としているとわかった場合は、オブジェクトを使うことができます。
これを解決するのがES2015/ES6のdestructuring assignment(分割代入構文)です。
Bad:
function createMenu(title, body, buttonText, cancellable) {
// ...
}Good:
function createMenu({ title, body, buttonText, cancellable }) {
// ...
}
createMenu({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
});これはとても重要なルールです。 関数をただ1つのことをやるように分離できた場合、それらを簡単にリファクタリングしたり、コードをかなりしっかりと読むことができます。
Bad:
function emailClients(clients) {
clients.forEach((client) => {
const clientRecord = database.lookup(client);
if (clientRecord.isActive()) {
email(client);
}
});
}Good:
function emailActiveClients(clients) {
clients
.filter(isActiveClient)
.forEach(email);
}
function isActiveClient(client) {
const clientRecord = database.lookup(client);
return clientRecord.isActive();
}Bad:
function addToDate(date, month) {
// ...
}
const date = new Date();
// It's hard to tell from the function name what is added
// 関数名からは何が追加されたのかがわかりにくい
addToDate(date, 1);Good:
function addMonthToDate(month, date) {
// ...
}
const date = new Date();
addMonthToDate(1, date);重複したコードを避けるために絶対にベストを尽くしてください。 重複したコードは、もし何かのロジックを変更しようとした場合、何か変更する場所が1つ以上あるという意味で悪です。
Bad:
function showDeveloperList(developers) {
developers.forEach((developer) => {
const expectedSalary = developer.calculateExpectedSalary();
const experience = developer.getExperience();
const githubLink = developer.getGithubLink();
const data = {
expectedSalary,
experience,
githubLink
};
render(data);
});
}
function showManagerList(managers) {
managers.forEach((manager) => {
const expectedSalary = manager.calculateExpectedSalary();
const experience = manager.getExperience();
const portfolio = manager.getMBAProjects();
const data = {
expectedSalary,
experience,
portfolio
};
render(data);
});
}Good:
function showEmployeeList(employees) {
employees.forEach((employee) => {
const expectedSalary = employee.calculateExpectedSalary();
const experience = employee.getExperience();
let portfolio;
switch (employee.type) {
case 'manager':
portfolio = employee.getMBAProjects();
break;
case 'developer':
portfolio = employee.getGithubLink();
break;
}
const data = {
expectedSalary,
experience,
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
// ユーザーは `body` キーを含めなくていい
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}
// configはこれと同じになります
// ...
}
createMenu(menuConfig); 似ているObject.assignと分割代入構文
Object.assignはオブジェクトのテンプレートを定義することに対し分割代入構文はその場でオブジェクトを作ること
フラグは、この関数が複数のことを行うことを利用者に伝えます。 関数は1つのことを行うべきです。関数が真偽値によって異なるコードの経路を経由する場合、その関数を分割してください。
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}`);
} ケースバイケースかと思います。
グローバルを汚染することはJavaScriptにおけるバッドプラクティスです。 なぜなら、他のライブラリをクラッシュさせるかもしれないし、あなたのAPIを使っているユーザーは、プロダクション環境で例外を受け取るまで、そのことについて何もわからないからです。
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:
function isDOMNodeNotPresent(node) {
// ...
}
if (!isDOMNodeNotPresent(node)) {
// なんらかの処理
}Good:
function isDOMNodePresent(node) {
// ...
}
if (isDOMNodePresent(node)) {
// 何もしない
}else{
// なんらかの処理
} 裏の裏は表みたいなややこしいことはしないで、基本を肯定で作りましょう
何もしないブロックがあってもいいと思う（後述のコメント削除）
モダンブラウザは、ランタイムの中で多くの最適化を行います。 何度も最適化を行なっているのであれば、それは時間の無駄です。ここにどこで最適化が不足するかをみるための良い資料があります。 可能であれば、それらが修正されるまでは、それらだけを最適化の対象としてください。
Bad:
// 古いブラウザにおいては、キャッシュされていない`list.length`はコストが掛かる
// なぜなら、`list.length`が再計算されるから。しかし、モダンプラウザでは最適化されている
for (let i = 0, len = list.length; i < len; i++) {
// ...
}Good:
for (let i = 0; i < list.length; i++) {
// ...
} c++は前置インクリメント(++i)にしたほうが良いとのこともありますがそちらも不要です。
基本構文
for(初期化; ループの継続条件; カウンタ変数の更新){
処理内容
} そもそもこのfor文を使う必要があるのか検討しましょう。
反復処理ならビルトイン関数で賄えるはずです。（array.map、array.filter、array.forEach）またはfor-in、for-of文。
ケース１、continue、breakが必要な場合、
ケース２、カウンタ変数を細かく制御したい場合
let t1 = [];
for(let i = 0; i < 20; i += 2){
t1.push(i);
}
console.log(t1.join(',')); // 0,2,4,6,8,10,12,14,16,18
let t2 = [];
for(let i = 1, x = 2, v = 21; x < v; i++, x = i * 3 - 1){
t2.push(x);
}
console.log(t2.join(',')); // 2,5,8,11,14,17,20使っていないコードは重複したコードと同じくらい悪いだけのものです。 コードがバージョン管理されていれば、復元するのも簡単です。
Bad:
function oldRequestModule(url) {
// ...
}
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');Good:
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');This can be accomplished through closures (for ES5 and below).
これはクロージャによって達成することができます。（ES5以前の場合）
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: undefinedGood:
function makeEmployee(name) {
return {
getName() {
return name;
},
};
}
const employee = makeEmployee('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これは、JavaScriptの中で非常に有用なパターンで、jQueryやLodashのような多くのライブラリの中でみることができます。
クラスの関数の中の全ての関数の終わりで、単純にthisを返すことで、クラスの中にあるメソッドをチェーンすることができます。
Bad:
class Car {
constructor() {
this.make = 'Honda';
this.model = 'Accord';
this.color = 'white';
}
setMake(make) {
this.make = make;
}
setModel(model) {
this.model = model;
}
setColor(color) {
this.color = color;
}
save() {
console.log(this.make, this.model, this.color);
}
}
const car = new Car();
car.setColor('pink');
car.setMake('Ford');
car.setModel('F-150');
car.save();Good:
class Car {
constructor() {
this.make = 'Honda';
this.model = 'Accord';
this.color = 'white';
}
setMake(make) {
this.make = make;
return this;
}
setModel(model) {
this.model = model;
return this;
}
setColor(color) {
this.color = color;
return this;
}
save() {
console.log(this.make, this.model, this.color);
return this;
}
}
const car = new Car()
.setColor('pink')
.setMake('Ford')
.setModel('F-150')
.save();コールバックは簡潔ではありません、そしてそれらは過剰な量のネストを引き起こします。 ES2015/ES6ではPromiseがグローバルに組み込まれています。それらを使いましょう！
Bad:
import { get } from 'request';
import { writeFile } from 'fs';
get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (requestErr, response) => {
if (requestErr) {
console.error(requestErr);
} else {
writeFile('article.html', response.body, (writeErr) => {
if (writeErr) {
console.error(writeErr);
} else {
console.log('File written');
}
});
}
});Good:
import { get } from 'request';
import { writeFile } from 'fs';
get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin')
.then((response) => {
return writeFile('article.html', response);
})
.then(() => {
console.log('File written');
})
.catch((err) => {
console.error(err);
});例外が発生することは良いことです！この意味は、ランタイムがあなたのプログラムが何かおかしいことを正常に突き止めたということです。 それは、関数の実行を直近のスタックで停止し、そのプロセスを停止し(ノード中)、コンソールのスタックトレースを通じてあなたに知らせてくれます。
例外を捕捉して何もしないということは、あなたがそのエラーを修正したり、エラーが言ったことに対応したりすることができません。
コンソール(console.log)にエラーを出力することは、頻繁にコンソール出力された海に埋もれてしまうため、それほど良いことではありません。
コードの一部を try/catch で囲うということは、そこでエラー発生するかもしれないということを意味します。
したがって、エラーが発生した時のために、なにかの対策をしたり、コードの行き先を作らなければなりません。
Bad:
try {
functionThatMightThrow();
} catch (error) {
console.log(error);
}Good:
try {
functionThatMightThrow();
} catch (error) {
// One option (more noisy than console.log):
console.error(error);
// Another option:
notifyUserOfError(error);
// Another option:
reportErrorToService(error);
// OR do all three!
}For the same reason you shouldn't ignore caught errors
from try/catch.
同じ理由により、try/catchにて発生した例外を無視するべきではありません。
Bad:
getdata()
.then((data) => {
functionThatMightThrow(data);
})
.catch((error) => {
console.log(error);
});Good:
getdata()
.then((data) => {
functionThatMightThrow(data);
})
.catch((error) => {
// One option (more noisy than console.log):
console.error(error);
// Another option:
notifyUserOfError(error);
// Another option:
reportErrorToService(error);
// OR do all three!
});組織単位でルールを作り、自動化ツールを１つ導入しましょう。
フォーマットについて議論することは、時間の無駄です。
理由は後述するugalify,minify化を参照
JavaScriptには型がありません。そのため大文字は変数や関数などについて多くのことを教えてくれます。 ここでのポイントは、組織あるいはあなたが決めたルールを守ることです。
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 {}この例のルール 定数はUPPERCASE + SNAKECASE 変数はCAMELCASE songs、artistsがSMALLCASE？にする
関数は他を呼び出す場合、それらをソースコードのなかの垂直方向で近くにおくようにしてください。 理想的には、呼び出し元を呼び出し先の上においてください。私たちは新聞のように、コードを上から下に読む傾向があります。 このため、あなたのコードをこのように読ませるようにしてください。
Bad:
class PerformanceReview {
constructor(employee) {
this.employee = employee;
}
lookupPeers() {
return db.lookup(this.employee, 'peers');
}
lookupManager() {
return db.lookup(this.employee, 'manager');
}
getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
perfReview() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
}
getManagerReview() {
const manager = this.lookupManager();
}
getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.perfReview();Good:
class PerformanceReview {
constructor(employee) {
this.employee = employee;
}
perfReview() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
}
getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
lookupPeers() {
return db.lookup(this.employee, 'peers');
}
getManagerReview() {
const manager = this.lookupManager();
}
lookupManager() {
return db.lookup(this.employee, 'manager');
}
getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.perfReview();Comments are an apology, not a requirement. Good code mostly documents itself.
コメントは弁明です、必須ではありません。良いコードは ほとんどが ドキュメントそのものです。
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;
}
}Version control exists for a reason. Leave old code in your history.
バージョン管理があることがその理由です。古いコードは履歴に残しましょう。
Bad:
doStuff();
// doOtherStuff();
// doSomeMoreStuff();
// doSoMuchStuff();Good:
doStuff();特に日記の様なコメント。
履歴を取得するためには git log を使ってください！
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;
}