clean-code-javascript - 🛁 JavaScript に適応したクリーンなコードの概念

(:bathtub: Clean Code concepts adapted for JavaScript)

Created at: 2016-11-26 06:25:41
Language: JavaScript
License: MIT

クリーンコードJavaScript

目次

  1. 序章
  2. 変数
  3. 機能
  4. オブジェクトとデータ構造
  5. クラス
  6. 個体
  7. テスト
  8. 同時実行
  9. エラー処理
  10. 書式設定
  11. コメント
  12. 翻訳

序章

コードを読むときにどれだけの罵倒を叫ぶかという、ソフトウェアの品質評価のユーモラスなイメージ

Robert C. Martin の著書Clean Codeのソフトウェア エンジニアリングの原則 を JavaScript に適応させたもの。これはスタイル ガイドではありません。これは、読みやすく、再利用可能で、リファクタリング可能なソフトウェアを JavaScriptで作成するためのガイド です。

ここに記載されているすべての原則に厳密に従う必要があるわけではありません。これらはガイドラインであり、それ以上のものではありませんが、 Clean Codeの作成者による長年の集合的な経験に基づいて成文化されたもの です。

私たちのソフトウェア エンジニアリング技術は 50 歳を少し超えたばかりですが、まだ多くのことを学んでいます。ソフトウェア アーキテクチャがアーキテクチャ自体と同じくらい古いものになると、従うべきルールが難しくなる可能性があります。今のところ、これらのガイドラインを、あなたとあなたのチームが作成する JavaScript コードの品質を評価するための試金石として役立ててください。

もう 1 つ: これらを知っていても、すぐに優れたソフトウェア開発者になるわけではありません。また、それらを何年も使用しても、間違いを犯さないというわけではありません。湿った粘土が最終的な形に形作られるように、コードのすべての部分は最初のドラフトとして始まります。最後に、同僚と一緒にレビューするときに、欠陥を削ぎ落とします。改善が必要な最初のドラフトで自分を責めないでください。代わりにコードを打ち負かしてください!

変数

意味があり発音しやすい変数名を使用する

悪い:

const yyyymmdstr = moment().format("YYYY/MM/DD");

良い:

const currentDate = moment().format("YYYY/MM/DD");

⬆トップに戻る

同じ型の変数には同じ語彙を使用する

悪い:

getUserInfo();
getClientData();
getCustomerRecord();

良い:

getUser();

⬆トップに戻る

検索可能な名前を使用する

私たちはこれまでに書いたよりも多くのコードを読むでしょう。私たちが書くコードが読みやすく、検索可能であることが重要です。プログラムを理解する上で意味のある変数に名前を付けないことで、読者を傷つけます。名前を検索可能にします。buddy.jsや ESLintなどのツールは 、名前のない定数を識別するのに役立ちます。

悪い:

// What the heck is 86400000 for?
setTimeout(blastOff, 86400000);

良い:

// Declare them as capitalized named constants.
const MILLISECONDS_PER_DAY = 60 * 60 * 24 * 1000; //86400000;

setTimeout(blastOff, MILLISECONDS_PER_DAY);

⬆トップに戻る

説明変数を使用する

悪い:

const address = "One Infinite Loop, Cupertino 95014";
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
saveCityZipCode(
  address.match(cityZipCodeRegex)[1],
  address.match(cityZipCodeRegex)[2]
);

良い:

const address = "One Infinite Loop, Cupertino 95014";
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
const [_, city, zipCode] = address.match(cityZipCodeRegex) || [];
saveCityZipCode(city, zipCode);

⬆トップに戻る

メンタルマッピングを避ける

明示的は暗黙的よりも優れています。

悪い:

const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(l => {
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  // Wait, what is `l` for again?
  dispatch(l);
});

良い:

const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(location => {
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  dispatch(location);
});

⬆トップに戻る

不要なコンテキストを追加しないでください

クラス/オブジェクト名が何かを伝えている場合は、変数名でそれを繰り返さないでください。

悪い:

const Car = {
  carMake: "Honda",
  carModel: "Accord",
  carColor: "Blue"
};

function paintCar(car, color) {
  car.carColor = color;
}

良い:

const Car = {
  make: "Honda",
  model: "Accord",
  color: "Blue"
};

function paintCar(car, color) {
  car.color = color;
}

⬆トップに戻る

短絡や条件の代わりにデフォルトのパラメーターを使用する

多くの場合、デフォルトのパラメーターは短絡よりもクリーンです。

undefined
それらを使用する場合、関数は引数のデフォルト値のみを提供することに注意してください。
''
、
""
、
false
、
null
、
0
、 などのその他の「偽の」値は
NaN
、デフォルト値に置き換えられません。

悪い:

function createMicrobrewery(name) {
  const breweryName = name || "Hipster Brew Co.";
  // ...
}

良い:

function createMicrobrewery(name = "Hipster Brew Co.") {
  // ...
}

⬆トップに戻る

機能

関数の引数 (理想的には 2 つ以下)

関数パラメーターの量を制限することは、関数のテストが容易になるため、非常に重要です。3 つを超えると、組み合わせの爆発が発生し、個別の引数ごとに多数の異なるケースをテストする必要があります。

1 つまたは 2 つの引数が理想的なケースであり、可能であれば 3 つを避ける必要があります。それ以上のものは統合する必要があります。通常、2 つ以上の引数がある場合、関数は多くのことをしようとしています。そうでない場合は、ほとんどの場合、より高いレベルのオブジェクトで引数として十分です。

JavaScript を使用すると、多くのクラス定型句を使用せずにオンザフライでオブジェクトを作成できるため、多くの引数が必要な場合はオブジェクトを使用できます。

関数が期待するプロパティを明確にするために、ES2015/ES6 分解構文を使用できます。これにはいくつかの利点があります。

  1. 関数シグネチャを見ると、どのプロパティが使用されているかがすぐにわかります。
  2. 名前付きパラメーターをシミュレートするために使用できます。
  3. 破壊は、関数に渡された引数オブジェクトの指定されたプリミティブ値も複製します。これにより、副作用を防ぐことができます。注: 引数オブジェクトから分解されたオブジェクトと配列は複製されません。
  4. リンターは、未使用のプロパティについて警告することができますが、これは破壊せずには不可能です。

悪い:

function createMenu(title, body, buttonText, cancellable) {
  // ...
}

createMenu("Foo", "Bar", "Baz", true);

良い:

function createMenu({ title, body, buttonText, cancellable }) {
  // ...
}

createMenu({
  title: "Foo",
  body: "Bar",
  buttonText: "Baz",
  cancellable: true
});

⬆トップに戻る

関数は 1 つのことを行う必要があります

これは、ソフトウェア エンジニアリングにおいて最も重要なルールです。関数が複数のことを行う場合、作成、テスト、推論が難しくなります。関数を 1 つのアクションだけに分離できれば、簡単にリファクタリングでき、コードはずっときれいに読めます。このガイドからこれ以外に何も得られなければ、多くの開発者よりも先に進むことができます。

悪い:

function emailClients(clients) {
  clients.forEach(client => {
    const clientRecord = database.lookup(client);
    if (clientRecord.isActive()) {
      email(client);
    }
  });
}

良い:

function emailActiveClients(clients) {
  clients.filter(isActiveClient).forEach(email);
}

function isActiveClient(client) {
  const clientRecord = database.lookup(client);
  return clientRecord.isActive();
}

⬆トップに戻る

関数名は何をするかを示すべきです

悪い:

function addToDate(date, month) {
  // ...
}

const date = new Date();

// It's hard to tell from the function name what is added
addToDate(date, 1);

良い:

function addMonthToDate(month, date) {
  // ...
}

const date = new Date();
addMonthToDate(1, date);

⬆トップに戻る

関数は 1 レベルの抽象化のみにする必要があります

抽象化のレベルが複数ある場合、通常、関数は過剰な処理を行います。関数を分割すると、再利用性が向上し、テストが容易になります。

悪い:

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...
  });
}

良い:

function parseBetterJSAlternative(code) {
  const tokens = tokenize(code);
  const syntaxTree = parse(tokens);
  syntaxTree.forEach(node => {
    // parse...
  });
}

function tokenize(code) {
  const REGEXES = [
    // ...
  ];

  const statements = code.split(" ");
  const tokens = [];
  REGEXES.forEach(REGEX => {
    statements.forEach(statement => {
      tokens.push(/* ... */);
    });
  });

  return tokens;
}

function parse(tokens) {
  const syntaxTree = [];
  tokens.forEach(token => {
    syntaxTree.push(/* ... */);
  });

  return syntaxTree;
}

⬆トップに戻る

重複コードを削除

コードの重複を避けるために最善を尽くしてください。ロジックを変更する必要がある場合に、何かを変更する場所が複数あることを意味するため、コードの重複は良くありません。

レストランを経営していて、すべてのトマト、タマネギ、ニンニク、スパイスなどの在庫を追跡している場合を想像してみてください。これを保持するリストが複数ある場合、トマトを使った料理を提供するときにすべてを更新する必要があります。それらの中で。リストが 1 つしかない場合、更新する場所は 1 つだけです。

多くの場合、2 つ以上のわずかに異なるものがあり、多くの共通点がありますが、それらの違いにより、ほとんど同じことを行う 2 つ以上の別個の関数が必要になるため、コードが重複することがあります。重複するコードを削除するということは、この一連のさまざまなものを 1 つの関数/モジュール/クラスだけで処理できる抽象化を作成することを意味します。

抽象化を正しく行うことは非常に重要です。そのため、クラスセクションで説明されている SOLID の原則に従う必要があります。悪い抽象化は重複コードよりも悪い可能性があるので、注意してください! そうは言っても、良い抽象化ができるなら、それをやってください!同じことを繰り返さないでください。そうしないと、1 つのことを変更したいときにいつでも複数の場所を更新していることに気付くでしょう。

悪い:

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);
  });
}

良い:

function showEmployeeList(employees) {
  employees.forEach(employee => {
    const expectedSalary = employee.calculateExpectedSalary();
    const experience = employee.getExperience();

    const data = {
      expectedSalary,
      experience
    };

    switch (employee.type) {
      case "manager":
        data.portfolio = employee.getMBAProjects();
        break;
      case "developer":
        data.githubLink = employee.getGithubLink();
        break;
    }

    render(data);
  });
}

⬆トップに戻る

Object.assign を使用してデフォルト オブジェクトを設定する

悪い:

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);

良い:

const menuConfig = {
  title: "Order",
  // User did not include 'body' key
  buttonText: "Send",
  cancellable: true
};

function createMenu(config) {
  let finalConfig = Object.assign(
    {
      title: "Foo",
      body: "Bar",
      buttonText: "Baz",
      cancellable: true
    },
    config
  );
  return finalConfig
  // config now equals: {title: "Order", body: "Bar", buttonText: "Send", cancellable: true}
  // ...
}

createMenu(menuConfig);

⬆トップに戻る

フラグを関数パラメーターとして使用しないでください

フラグは、この関数が複数のことを行うことをユーザーに伝えます。関数は 1 つのことを行う必要があります。ブール値に基づいて異なるコード パスに従っている場合は、関数を分割します。

悪い:

function createFile(name, temp) {
  if (temp) {
    fs.create(`./temp/${name}`);
  } else {
    fs.create(name);
  }
}

良い:

function createFile(name) {
  fs.create(name);
}

function createTempFile(name) {
  createFile(`./temp/${name}`);
}

⬆トップに戻る

副作用を避ける (パート 1)

関数は、値を取得して別の値を返す以外のことを行うと、副作用を引き起こします。副作用は、ファイルへの書き込み、グローバル変数の変更、または誤ってすべてのお金を見知らぬ人に配線する可能性があります.

さて、場合によってはプログラムに副作用を持たせる必要があります。前の例と同様に、ファイルへの書き込みが必要になる場合があります。あなたがしたいことは、これを行っている場所を一元化することです。特定のファイルに書き込む関数やクラスをいくつか持たないでください。それを行う1つのサービスを用意してください。唯一無二。

主なポイントは、オブジェクト間で構造を持たずに状態を共有する、何からでも書き込み可能な可変データ型を使用する、副作用が発生する場所を一元化しないなどのよくある落とし穴を避けることです。これができれば、他の大多数のプログラマーよりも幸せになれるでしょう。

悪い:

// 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'];

良い:

function splitIntoFirstAndLastName(name) {
  return name.split(" ");
}

const name = "Ryan McDermott";
const newName = splitIntoFirstAndLastName(name);

console.log(name); // 'Ryan McDermott';
console.log(newName); // ['Ryan', 'McDermott'];

⬆トップに戻る

副作用を避ける (パート 2)

JavaScript では、一部の値は変更不可 (不変) であり、一部は変更可能 (ミュータブル) です。オブジェクトと配列は 2 種類の可変値であるため、関数にパラメーターとして渡すときは慎重に扱うことが重要です。JavaScript 関数は、オブジェクトのプロパティを変更したり、他の場所で簡単にバグを引き起こす可能性のある配列の内容を変更したりできます。

ショッピング カートを表す配列パラメーターを受け取る関数があるとします。関数がそのショッピング カート配列に変更を加えた場合 (購入するアイテムを追加するなど)、同じ

cart
配列を使用する他のすべての関数がこの追加の影響を受けます。それは素晴らしいことかもしれませんが、悪いこともあります。悪い状況を想像してみましょう:

ユーザーが「購入」ボタンをクリックすると

purchase
、ネットワーク要求を生成して
cart
アレイをサーバーに送信する関数が呼び出されます。ネットワーク接続が悪いため、
purchase
関数はリクエストを再試行し続ける必要があります。では、ネットワーク リクエストが開始される前に、ユーザーが実際には欲しくないアイテムの [カートに追加] ボタンを誤ってクリックしてしまったらどうなるでしょうか。
cart
それが発生し、ネットワーク リクエストが開始された場合、その購入関数は、配列が変更されたため、誤って追加されたアイテムを送信します。

優れた解決策は、

addItemToCart
関数が常に を複製し
cart
、編集して、複製を返すことです。これにより、古いショッピング カートをまだ使用している関数が変更の影響を受けないことが保証されます。

このアプローチについて言及する 2 つの注意事項:

  1. 入力オブジェクトを実際に変更したい場合もあるかもしれませんが、このプログラミング手法を採用すると、そのようなケースは非常にまれであることがわかります。ほとんどのものは、副作用がないようにリファクタリングできます!

  2. 大きなオブジェクトのクローンを作成すると、パフォーマンスの点で非常にコストがかかる可能性があります。幸いなことに、これは実際には大きな問題ではありません。なぜなら 、この種のプログラミング アプローチを高速にし、オブジェクトや配列を手動で複製する場合ほどメモリを消費しない優れたライブラリがあるからです。

悪い:

const addItemToCart = (cart, item) => {
  cart.push({ item, date: Date.now() });
};

良い:

const addItemToCart = (cart, item) => {
  return [...cart, { item, date: Date.now() }];
};

⬆トップに戻る

グローバル関数に書き込まないでください

グローバルを汚染することは、JavaScript では悪い習慣です。別のライブラリと競合する可能性があり、API のユーザーは、本番環境で例外が発生するまでは賢明ではありません。例を考えてみましょう。JavaScript のネイティブ Array メソッドを拡張して、

diff
2 つの配列の違いを表示できるメソッドを作成したい場合はどうでしょうか? 新しい関数を に書き込むことはできます
Array.prototype
が、同じことをしようとした別のライブラリと衝突する可能性があります。その他のライブラリが
diff
、配列の最初と最後の要素の違いを見つけるために使用されていたとしたらどうでしょうか? これが、ES2015/ES6 クラスを使用して
Array
グローバルを拡張する方がはるかに優れている理由です。

悪い:

Array.prototype.diff = function diff(comparisonArray) {
  const hash = new Set(comparisonArray);
  return this.filter(elem => !hash.has(elem));
};

良い:

class SuperArray extends Array {
  diff(comparisonArray) {
    const hash = new Set(comparisonArray);
    return this.filter(elem => !hash.has(elem));
  }
}

⬆トップに戻る

命令型プログラミングよりも関数型プログラミングを好む

JavaScript は Haskell のような関数型言語ではありませんが、関数型の風味があります。関数型言語は、よりクリーンで簡単にテストできます。可能な限り、このスタイルのプログラミングを優先してください。

悪い:

const programmerOutput = [
  {
    name: "Uncle Bobby",
    linesOfCode: 500
  },
  {
    name: "Suzie Q",
    linesOfCode: 1500
  },
  {
    name: "Jimmy Gosling",
    linesOfCode: 150
  },
  {
    name: "Gracie Hopper",
    linesOfCode: 1000
  }
];

let totalOutput = 0;

for (let i = 0; i < programmerOutput.length; i++) {
  totalOutput += programmerOutput[i].linesOfCode;
}

良い:

const programmerOutput = [
  {
    name: "Uncle Bobby",
    linesOfCode: 500
  },
  {
    name: "Suzie Q",
    linesOfCode: 1500
  },
  {
    name: "Jimmy Gosling",
    linesOfCode: 150
  },
  {
    name: "Gracie Hopper",
    linesOfCode: 1000
  }
];

const totalOutput = programmerOutput.reduce(
  (totalLines, output) => totalLines + output.linesOfCode,
  0
);

⬆トップに戻る

条件をカプセル化する

悪い:

if (fsm.state === "fetching" && isEmpty(listNode)) {
  // ...
}

良い:

function shouldShowSpinner(fsm, listNode) {
  return fsm.state === "fetching" && isEmpty(listNode);
}

if (shouldShowSpinner(fsmInstance, listNodeInstance)) {
  // ...
}

⬆トップに戻る

否定的な条件を避ける

悪い:

function isDOMNodeNotPresent(node) {
  // ...
}

if (!isDOMNodeNotPresent(node)) {
  // ...
}

良い:

function isDOMNodePresent(node) {
  // ...
}

if (isDOMNodePresent(node)) {
  // ...
}

⬆トップに戻る

条件を避ける

これは不可能な作業のようです。

if
これを最初に聞いたとき、ほとんどの人は「声明なしでどうやって何かをすることになっているのですか?」と言います。答えは、多くの場合、ポリモーフィズムを使用して同じタスクを達成できるということです。2 番目の質問は通常、「それは素晴らしいことですが、なぜそれを行う必要があるのでしょうか?」というものです。その答えは、以前に学んだクリーンなコードの概念です。関数は 1 つのことだけを行うべきです。ステートメントを持つクラスと関数がある場合
if
、関数が複数のことを行うことをユーザーに伝えています。1 つのことだけを行うことを忘れないでください。

悪い:

class Airplane {
  // ...
  getCruisingAltitude() {
    switch (this.type) {
      case "777":
        return this.getMaxAltitude() - this.getPassengerCount();
      case "Air Force One":
        return this.getMaxAltitude();
      case "Cessna":
        return this.getMaxAltitude() - this.getFuelExpenditure();
    }
  }
}

良い:

class Airplane {
  // ...
}

class Boeing777 extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude() - this.getPassengerCount();
  }
}

class AirForceOne extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude();
  }
}

class Cessna extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude() - this.getFuelExpenditure();
  }
}

⬆トップに戻る

型チェックを避ける (パート 1)

JavaScript は型指定されていません。つまり、関数は任意の型の引数を取ることができます。時々、この自由に噛まれて、関数で型チェックをしたくなることがあります。これを回避する方法はたくさんあります。最初に考慮すべきことは、一貫した API です。

悪い:

function travelToTexas(vehicle) {
  if (vehicle instanceof Bicycle) {
    vehicle.pedal(this.currentLocation, new Location("texas"));
  } else if (vehicle instanceof Car) {
    vehicle.drive(this.currentLocation, new Location("texas"));
  }
}

良い:

function travelToTexas(vehicle) {
  vehicle.move(this.currentLocation, new Location("texas"));
}

⬆トップに戻る

型チェックを避ける (パート 2)

文字列や整数などの基本的なプリミティブ値を扱っていて、ポリモーフィズムを使用できないが、それでも型チェックの必要性を感じている場合は、TypeScript の使用を検討する必要があります。これは、標準の JavaScript 構文に加えて静的型付けを提供するため、通常の JavaScript の優れた代替手段です。通常の JavaScript を手動で型チェックする際の問題は、それをうまく行うには余分な言い回しが必要になるため、得られる偽の「型安全性」が失われた可読性を補えないことです。JavaScript をきれいに保ち、適切なテストを作成し、適切なコード レビューを行ってください。それ以外の場合は、すべてを TypeScript を使用して行います (これは、私が言ったように、優れた代替手段です!)。

悪い:

function combine(val1, val2) {
  if (
    (typeof val1 === "number" && typeof val2 === "number") ||
    (typeof val1 === "string" && typeof val2 === "string")
  ) {
    return val1 + val2;
  }

  throw new Error("Must be of type String or Number");
}

良い:

function combine(val1, val2) {
  return val1 + val2;
}

⬆トップに戻る

最適化しすぎない

最新のブラウザーは、実行時に内部で多くの最適化を行います。多くの場合、最適化を行っている場合は、時間を無駄にしているだけです。 最適化が欠けている場所を確認するための優れたリソースがあります。可能な場合は修正されるまでの間、それらをターゲットにします。

悪い:

// On old browsers, each iteration with uncached `list.length` would be costly
// because of `list.length` recomputation. In modern browsers, this is optimized.
for (let i = 0, len = list.length; i < len; i++) {
  // ...
}

良い:

for (let i = 0; i < list.length; i++) {
  // ...
}

⬆トップに戻る

デッドコードを削除

デッド コードは、重複コードと同じくらい悪いものです。コードベースに保持する理由はありません。呼び出されていない場合は、削除してください。それでも必要な場合は、バージョン履歴で安全に保管されます。

悪い:

function oldRequestModule(url) {
  // ...
}

function newRequestModule(url) {
  // ...
}

const req = newRequestModule;
inventoryTracker("apples", req, "www.inventory-awesome.io");

良い:

function newRequestModule(url) {
  // ...
}

const req = newRequestModule;
inventoryTracker("apples", req, "www.inventory-awesome.io");

⬆トップに戻る

オブジェクトとデータ構造

ゲッターとセッターを使用する

単にオブジェクトのプロパティを探すよりも、getter と setter を使用してオブジェクトのデータにアクセスするほうがよい場合があります。"どうして?" あなたは尋ねるかもしれません。さて、ここに理由の整理されていないリストがあります:

  • When you want to do more beyond getting an object property, you don't have to look up and change every accessor in your codebase.
  • Makes adding validation simple when doing a
    set
    .
  • Encapsulates the internal representation.
  • Easy to add logging and error handling when getting and setting.
  • You can lazy load your object's properties, let's say getting it from a server.

Bad:

function makeBankAccount() {
  // ...

  return {
    balance: 0
    // ...
  };
}

const account = makeBankAccount();
account.balance = 100;

Good:

function makeBankAccount() {
  // this one is private
  let balance = 0;

  // a "getter", made public via the returned object below
  function getBalance() {
    return balance;
  }

  // a "setter", made public via the returned object below
  function setBalance(amount) {
    // ... validate before updating the balance
    balance = amount;
  }

  return {
    // ...
    getBalance,
    setBalance
  };
}

const account = makeBankAccount();
account.setBalance(100);

⬆ back to top

Make objects have private members

This can be accomplished through closures (for ES5 and below).

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:

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

⬆ back to top

Classes

Prefer ES2015/ES6 classes over ES5 plain functions

It's very difficult to get readable class inheritance, construction, and method definitions for classical ES5 classes. If you need inheritance (and be aware that you might not), then prefer ES2015/ES6 classes. However, prefer small functions over classes until you find yourself needing larger and more complex objects.

Bad:

const Animal = function(age) {
  if (!(this instanceof Animal)) {
    throw new Error("Instantiate Animal with `new`");
  }

  this.age = age;
};

Animal.prototype.move = function move() {};

const Mammal = function(age, furColor) {
  if (!(this instanceof Mammal)) {
    throw new Error("Instantiate Mammal with `new`");
  }

  Animal.call(this, age);
  this.furColor = furColor;
};

Mammal.prototype = Object.create(Animal.prototype);
Mammal.prototype.constructor = Mammal;
Mammal.prototype.liveBirth = function liveBirth() {};

const Human = function(age, furColor, languageSpoken) {
  if (!(this instanceof Human)) {
    throw new Error("Instantiate Human with `new`");
  }

  Mammal.call(this, age, furColor);
  this.languageSpoken = languageSpoken;
};

Human.prototype = Object.create(Mammal.prototype);
Human.prototype.constructor = Human;
Human.prototype.speak = function speak() {};

Good:

class Animal {
  constructor(age) {
    this.age = age;
  }

  move() {
    /* ... */
  }
}

class Mammal extends Animal {
  constructor(age, furColor) {
    super(age);
    this.furColor = furColor;
  }

  liveBirth() {
    /* ... */
  }
}

class Human extends Mammal {
  constructor(age, furColor, languageSpoken) {
    super(age, furColor);
    this.languageSpoken = languageSpoken;
  }

  speak() {
    /* ... */
  }
}

⬆ back to top

Use method chaining

This pattern is very useful in JavaScript and you see it in many libraries such as jQuery and Lodash. It allows your code to be expressive, and less verbose. For that reason, I say, use method chaining and take a look at how clean your code will be. In your class functions, simply return

this
at the end of every function, and you can chain further class methods onto it.

Bad:

class Car {
  constructor(make, model, color) {
    this.make = make;
    this.model = model;
    this.color = color;
  }

  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("Ford", "F-150", "red");
car.setColor("pink");
car.save();

Good:

class Car {
  constructor(make, model, color) {
    this.make = make;
    this.model = model;
    this.color = color;
  }

  setMake(make) {
    this.make = make;
    // NOTE: Returning this for chaining
    return this;
  }

  setModel(model) {
    this.model = model;
    // NOTE: Returning this for chaining
    return this;
  }

  setColor(color) {
    this.color = color;
    // NOTE: Returning this for chaining
    return this;
  }

  save() {
    console.log(this.make, this.model, this.color);
    // NOTE: Returning this for chaining
    return this;
  }
}

const car = new Car("Ford", "F-150", "red").setColor("pink").save();

⬆ back to top

Prefer composition over inheritance

As stated famously in Design Patterns by the Gang of Four, you should prefer composition over inheritance where you can. There are lots of good reasons to use inheritance and lots of good reasons to use composition. The main point for this maxim is that if your mind instinctively goes for inheritance, try to think if composition could model your problem better. In some cases it can.

You might be wondering then, "when should I use inheritance?" It depends on your problem at hand, but this is a decent list of when inheritance makes more sense than composition:

  1. Your inheritance represents an "is-a" relationship and not a "has-a" relationship (Human->Animal vs. User->UserDetails).
  2. You can reuse code from the base classes (Humans can move like all animals).
  3. You want to make global changes to derived classes by changing a base class. (Change the caloric expenditure of all animals when they move).

Bad:

class Employee {
  constructor(name, email) {
    this.name = name;
    this.email = email;
  }

  // ...
}

// Bad because Employees "have" tax data. EmployeeTaxData is not a type of Employee
class EmployeeTaxData extends Employee {
  constructor(ssn, salary) {
    super();
    this.ssn = ssn;
    this.salary = salary;
  }

  // ...
}

Good:

class EmployeeTaxData {
  constructor(ssn, salary) {
    this.ssn = ssn;
    this.salary = salary;
  }

  // ...
}

class Employee {
  constructor(name, email) {
    this.name = name;
    this.email = email;
  }

  setTaxData(ssn, salary) {
    this.taxData = new EmployeeTaxData(ssn, salary);
  }
  // ...
}

⬆ back to top

SOLID

Single Responsibility Principle (SRP)

As stated in Clean Code, "There should never be more than one reason for a class to change". It's tempting to jam-pack a class with a lot of functionality, like when you can only take one suitcase on your flight. The issue with this is that your class won't be conceptually cohesive and it will give it many reasons to change. Minimizing the amount of times you need to change a class is important. It's important because if too much functionality is in one class and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase.

Bad:

class UserSettings {
  constructor(user) {
    this.user = user;
  }

  changeSettings(settings) {
    if (this.verifyCredentials()) {
      // ...
    }
  }

  verifyCredentials() {
    // ...
  }
}

Good:

class UserAuth {
  constructor(user) {
    this.user = user;
  }

  verifyCredentials() {
    // ...
  }
}

class UserSettings {
  constructor(user) {
    this.user = user;
    this.auth = new UserAuth(user);
  }

  changeSettings(settings) {
    if (this.auth.verifyCredentials()) {
      // ...
    }
  }
}

⬆ back to top

Open/Closed Principle (OCP)

As stated by Bertrand Meyer, "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification." What does that mean though? This principle basically states that you should allow users to add new functionalities without changing existing code.

Bad:

class AjaxAdapter extends Adapter {
  constructor() {
    super();
    this.name = "ajaxAdapter";
  }
}

class NodeAdapter extends Adapter {
  constructor() {
    super();
    this.name = "nodeAdapter";
  }
}

class HttpRequester {
  constructor(adapter) {
    this.adapter = adapter;
  }

  fetch(url) {
    if (this.adapter.name === "ajaxAdapter") {
      return makeAjaxCall(url).then(response => {
        // transform response and return
      });
    } else if (this.adapter.name === "nodeAdapter") {
      return makeHttpCall(url).then(response => {
        // transform response and return
      });
    }
  }
}

function makeAjaxCall(url) {
  // request and return promise
}

function makeHttpCall(url) {
  // request and return promise
}

Good:

class AjaxAdapter extends Adapter {
  constructor() {
    super();
    this.name = "ajaxAdapter";
  }

  request(url) {
    // request and return promise
  }
}

class NodeAdapter extends Adapter {
  constructor() {
    super();
    this.name = "nodeAdapter";
  }

  request(url) {
    // request and return promise
  }
}

class HttpRequester {
  constructor(adapter) {
    this.adapter = adapter;
  }

  fetch(url) {
    return this.adapter.request(url).then(response => {
      // transform response and return
    });
  }
}

⬆ back to top

Liskov Substitution Principle (LSP)

This is a scary term for a very simple concept. It's formally defined as "If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may substitute objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.)." That's an even scarier definition.

The best explanation for this is if you have a parent class and a child class, then the base class and child class can be used interchangeably without getting incorrect results. This might still be confusing, so let's take a look at the classic Square-Rectangle example. Mathematically, a square is a rectangle, but if you model it using the "is-a" relationship via inheritance, you quickly get into trouble.

Bad:

class Rectangle {
  constructor() {
    this.width = 0;
    this.height = 0;
  }

  setColor(color) {
    // ...
  }

  render(area) {
    // ...
  }

  setWidth(width) {
    this.width = width;
  }

  setHeight(height) {
    this.height = height;
  }

  getArea() {
    return this.width * this.height;
  }
}

class Square extends Rectangle {
  setWidth(width) {
    this.width = width;
    this.height = width;
  }

  setHeight(height) {
    this.width = height;
    this.height = height;
  }
}

function renderLargeRectangles(rectangles) {
  rectangles.forEach(rectangle => {
    rectangle.setWidth(4);
    rectangle.setHeight(5);
    const area = rectangle.getArea(); // BAD: Returns 25 for Square. Should be 20.
    rectangle.render(area);
  });
}

const rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles(rectangles);

Good:

class Shape {
  setColor(color) {
    // ...
  }

  render(area) {
    // ...
  }
}

class Rectangle extends Shape {
  constructor(width, height) {
    super();
    this.width = width;
    this.height = height;
  }

  getArea() {
    return this.width * this.height;
  }
}

class Square extends Shape {
  constructor(length) {
    super();
    this.length = length;
  }

  getArea() {
    return this.length * this.length;
  }
}

function renderLargeShapes(shapes) {
  shapes.forEach(shape => {
    const area = shape.getArea();
    shape.render(area);
  });
}

const shapes = [new Rectangle(4, 5), new Rectangle(4, 5), new Square(5)];
renderLargeShapes(shapes);

⬆ back to top

Interface Segregation Principle (ISP)

JavaScript doesn't have interfaces so this principle doesn't apply as strictly as others. However, it's important and relevant even with JavaScript's lack of type system.

ISP states that "Clients should not be forced to depend upon interfaces that they do not use." Interfaces are implicit contracts in JavaScript because of duck typing.

A good example to look at that demonstrates this principle in JavaScript is for classes that require large settings objects. Not requiring clients to setup huge amounts of options is beneficial, because most of the time they won't need all of the settings. Making them optional helps prevent having a "fat interface".

Bad:

class DOMTraverser {
  constructor(settings) {
    this.settings = settings;
    this.setup();
  }

  setup() {
    this.rootNode = this.settings.rootNode;
    this.settings.animationModule.setup();
  }

  traverse() {
    // ...
  }
}

const $ = new DOMTraverser({
  rootNode: document.getElementsByTagName("body"),
  animationModule() {} // Most of the time, we won't need to animate when traversing.
  // ...
});

Good:

class DOMTraverser {
  constructor(settings) {
    this.settings = settings;
    this.options = settings.options;
    this.setup();
  }

  setup() {
    this.rootNode = this.settings.rootNode;
    this.setupOptions();
  }

  setupOptions() {
    if (this.options.animationModule) {
      // ...
    }
  }

  traverse() {
    // ...
  }
}

const $ = new DOMTraverser({
  rootNode: document.getElementsByTagName("body"),
  options: {
    animationModule() {}
  }
});

⬆ back to top

Dependency Inversion Principle (DIP)

This principle states two essential things:

  1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
  2. Abstractions should not depend upon details. Details should depend on abstractions.

This can be hard to understand at first, but if you've worked with AngularJS, you've seen an implementation of this principle in the form of Dependency Injection (DI). While they are not identical concepts, DIP keeps high-level modules from knowing the details of its low-level modules and setting them up. It can accomplish this through DI. A huge benefit of this is that it reduces the coupling between modules. Coupling is a very bad development pattern because it makes your code hard to refactor.

As stated previously, JavaScript doesn't have interfaces so the abstractions that are depended upon are implicit contracts. That is to say, the methods and properties that an object/class exposes to another object/class. In the example below, the implicit contract is that any Request module for an

InventoryTracker
will have a
requestItems
method.

Bad:

class InventoryRequester {
  constructor() {
    this.REQ_METHODS = ["HTTP"];
  }

  requestItem(item) {
    // ...
  }
}

class InventoryTracker {
  constructor(items) {
    this.items = items;

    // BAD: We have created a dependency on a specific request implementation.
    // We should just have requestItems depend on a request method: `request`
    this.requester = new InventoryRequester();
  }

  requestItems() {
    this.items.forEach(item => {
      this.requester.requestItem(item);
    });
  }
}

const inventoryTracker = new InventoryTracker(["apples", "bananas"]);
inventoryTracker.requestItems();

Good:

class InventoryTracker {
  constructor(items, requester) {
    this.items = items;
    this.requester = requester;
  }

  requestItems() {
    this.items.forEach(item => {
      this.requester.requestItem(item);
    });
  }
}

class InventoryRequesterV1 {
  constructor() {
    this.REQ_METHODS = ["HTTP"];
  }

  requestItem(item) {
    // ...
  }
}

class InventoryRequesterV2 {
  constructor() {
    this.REQ_METHODS = ["WS"];
  }

  requestItem(item) {
    // ...
  }
}

// By constructing our dependencies externally and injecting them, we can easily
// substitute our request module for a fancy new one that uses WebSockets.
const inventoryTracker = new InventoryTracker(
  ["apples", "bananas"],
  new InventoryRequesterV2()
);
inventoryTracker.requestItems();

⬆ back to top

Testing

Testing is more important than shipping. If you have no tests or an inadequate amount, then every time you ship code you won't be sure that you didn't break anything. Deciding on what constitutes an adequate amount is up to your team, but having 100% coverage (all statements and branches) is how you achieve very high confidence and developer peace of mind. This means that in addition to having a great testing framework, you also need to use a good coverage tool.

There's no excuse to not write tests. There are plenty of good JS test frameworks, so find one that your team prefers. When you find one that works for your team, then aim to always write tests for every new feature/module you introduce. If your preferred method is Test Driven Development (TDD), that is great, but the main point is to just make sure you are reaching your coverage goals before launching any feature, or refactoring an existing one.

Single concept per test

Bad:

import assert from "assert";

describe("MomentJS", () => {
  it("handles date boundaries", () => {
    let date;

    date = new MomentJS("1/1/2015");
    date.addDays(30);
    assert.equal("1/31/2015", date);

    date = new MomentJS("2/1/2016");
    date.addDays(28);
    assert.equal("02/29/2016", date);

    date = new MomentJS("2/1/2015");
    date.addDays(28);
    assert.equal("03/01/2015", date);
  });
});

Good:

import assert from "assert";

describe("MomentJS", () => {
  it("handles 30-day months", () => {
    const date = new MomentJS("1/1/2015");
    date.addDays(30);
    assert.equal("1/31/2015", date);
  });

  it("handles leap year", () => {
    const date = new MomentJS("2/1/2016");
    date.addDays(28);
    assert.equal("02/29/2016", date);
  });

  it("handles non-leap year", () => {
    const date = new MomentJS("2/1/2015");
    date.addDays(28);
    assert.equal("03/01/2015", date);
  });
});

⬆ back to top

Concurrency

Use Promises, not callbacks

Callbacks aren't clean, and they cause excessive amounts of nesting. With ES2015/ES6, Promises are a built-in global type. Use them!

Bad:

import { get } from "request";
import { writeFile } from "fs";

get(
  "https://en.wikipedia.org/wiki/Robert_Cecil_Martin",
  (requestErr, response, body) => {
    if (requestErr) {
      console.error(requestErr);
    } else {
      writeFile("article.html", body, writeErr => {
        if (writeErr) {
          console.error(writeErr);
        } else {
          console.log("File written");
        }
      });
    }
  }
);

Good:

import { get } from "request-promise";
import { writeFile } from "fs-extra";

get("https://en.wikipedia.org/wiki/Robert_Cecil_Martin")
  .then(body => {
    return writeFile("article.html", body);
  })
  .then(() => {
    console.log("File written");
  })
  .catch(err => {
    console.error(err);
  });

⬆ back to top

Async/Await are even cleaner than Promises

Promises are a very clean alternative to callbacks, but ES2017/ES8 brings async and await which offer an even cleaner solution. All you need is a function that is prefixed in an

async
keyword, and then you can write your logic imperatively without a
then
chain of functions. Use this if you can take advantage of ES2017/ES8 features today!

Bad:

import { get } from "request-promise";
import { writeFile } from "fs-extra";

get("https://en.wikipedia.org/wiki/Robert_Cecil_Martin")
  .then(body => {
    return writeFile("article.html", body);
  })
  .then(() => {
    console.log("File written");
  })
  .catch(err => {
    console.error(err);
  });

Good:

import { get } from "request-promise";
import { writeFile } from "fs-extra";

async function getCleanCodeArticle() {
  try {
    const body = await get(
      "https://en.wikipedia.org/wiki/Robert_Cecil_Martin"
    );
    await writeFile("article.html", body);
    console.log("File written");
  } catch (err) {
    console.error(err);
  }
}

getCleanCodeArticle()

⬆ back to top

Error Handling

Thrown errors are a good thing! They mean the runtime has successfully identified when something in your program has gone wrong and it's letting you know by stopping function execution on the current stack, killing the process (in Node), and notifying you in the console with a stack trace.

Don't ignore caught errors

Doing nothing with a caught error doesn't give you the ability to ever fix or react to said error. Logging the error to the console (

console.log
) isn't much better as often times it can get lost in a sea of things printed to the console. If you wrap any bit of code in a
try/catch
it means you think an error may occur there and therefore you should have a plan, or create a code path, for when it occurs.

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!
}

Don't ignore rejected promises

For the same reason you shouldn't ignore caught errors from

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!
  });

⬆ back to top

Formatting

Formatting is subjective. Like many rules herein, there is no hard and fast rule that you must follow. The main point is DO NOT ARGUE over formatting. There are tons of tools to automate this. Use one! It's a waste of time and money for engineers to argue over formatting.

For things that don't fall under the purview of automatic formatting (indentation, tabs vs. spaces, double vs. single quotes, etc.) look here for some guidance.

Use consistent capitalization

JavaScript is untyped, so capitalization tells you a lot about your variables, functions, etc. These rules are subjective, so your team can choose whatever they want. The point is, no matter what you all choose, just be consistent.

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 {}

⬆ back to top

Function callers and callees should be close

If a function calls another, keep those functions vertically close in the source file. Ideally, keep the caller right above the callee. We tend to read code from top-to-bottom, like a newspaper. Because of this, make your code read that way.

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();

⬆ back to top

Comments

Only comment things that have business logic complexity.

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;
  }
}

⬆ back to top

Don't leave commented out code in your codebase

Version control exists for a reason. Leave old code in your history.

Bad:

doStuff();
// doOtherStuff();
// doSomeMoreStuff();
// doSoMuchStuff();

Good:

doStuff();

⬆ back to top

Don't have journal comments

Remember, use version control! There's no need for dead code, commented code, and especially journal comments. Use

git log
to get history!

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;
}

⬆ back to top

Avoid positional markers

They usually just add noise. Let the functions and variable names along with the proper indentation and formatting give the visual structure to your code.

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() {
  // ...
};

⬆ back to top

Translation

This is also available in other languages:

⬆トップに戻る