SIMPLE

よくあったコードレビューの指摘ポイントまとめ(JavaScript/TypeScript)

コードレビューをする上でよくあがる指摘をまとめてみました。

1. テンプレートリテラルを使おう

+ を使った文字列結合は、特に数値と連結する場面でバグの原因になりやすいです。
例えば userId + "さん" だと、userIdnullundefined のときに意図しない文字列になることがあります。
テンプレートリテラル( ${userId}さん )を使えば、可読性も安全性もアップします。


2. 早期 return でネストを浅く

複雑な条件分岐が増えてくると、ネストがどんどん深くなって読みづらいコードになります。
その場合は、ガード節(早期 return) を使って、処理の本筋をスッキリ見せるようにしましょう。

// dataに値がなかったり、値チェックにひっかかる場合は早期returnする

// NG(ネストが深い) 
if (data) { 
  if (data.valid) { 
    // 処理 
  } 
} 

// OK(早期 return) 
if (!data || !data.valid) return; // 処理

3. 初期値の設定で分岐を減らす

関数の引数やローカル変数にデフォルト値を設定することで、余計なif文やnullチェックを避けられます。

function greet(name = "ゲスト") { 
  console.log(`こんにちは、${name}さん!`); 
}

4. 配列処理には find, some, includes を活用

for文だけで書くよりも、意図が明確に伝わる配列メソッドを使う方が読みやすいです。

// NG なにをやっているのか読み解かないといけない
let found = false; 
for (let i = 0; i < arr.length; i++) { 
  if (arr[i] === target) { 
    found = true; break; 
  } 
} 

// OK 1発で「targetが含まれている」か意図がわかる
const found = arr.includes(target);

5. for-of や for-in を使って簡潔に書こう

forEach は非同期処理に弱く、await を使っても期待通りに順番に動作しないことがあります。

for-of を使うと、await が自然に使えるのでおすすめです。

※言語によっては非同期でなくても順番が保証されないケースもあるので注意

// NG 
arr.forEach(async item => { 
  await doSomething(item); // 順番に動かない 
  }
); 

// OK 
for (const item of arr) { 
  await doSomething(item); // 順番に処理される
} 

6. Promise.allSettled を活用しよう

複数の非同期処理を同時に走らせるとき、どれか一つでも失敗したら全体が止まる Promise.all ではなく、
全ての結果を受け取りたい 場面では Promise.allSettled が便利です。

// p1, p2, p3という非同期メソッドがある場合

// NG
await p1();
await p2();
await p3();

// OK
const results = await Promise.allSettled([p1, p2, p3]);
results.forEach(result => {
  if (result.status === "fulfilled") {
    console.log("成功:", result.value);
  } else {
    console.warn("失敗:", result.reason);
  }
});

7. 0埋めは padStart を使おう

日時や連番のフォーマットでよく使うので活用しましょう。

0埋めでなくても特定の文字で埋めることもできます。

const id = String(5).padStart(3, "0"); // "005"

8. 大量データ処理では連想配列(オブジェクトやMap)を使おう

巨大な配列から要素を検索する場合、毎回 findfilter を使うと 最悪全検索O(n) の処理になります。
key-value の形であらかじめ連想配列にしておけば、1発検索O(1) でアクセス可能です(DBの主キーと同じ考え方)。

Mapも高速なので是非活用してください。

const users = [
  { id: 'u001', name: '太郎' },
  { id: 'u002', name: '花子' },
  { id: 'u003', name: '次郎' },
];

// 検索対象
const targetId = 'u003';

// NG 配列の最後のデータを検索するので全検索される
const user = users.find(user => user.id === targetId);

console.log(user); // { id: 'u003', name: '次郎' }
const usersArray = [
  { id: 'u001', name: '太郎' },
  { id: 'u002', name: '花子' },
  { id: 'u003', name: '次郎' },
];

// Mapに変換
const usersMap = new Map(usersArray.map(user => [user.id, user]));

// 検索対象
const targetId = 'u003';

// OK データがどこにあろうが1発で検索できる
const user = usersMap.get(targetId);

console.log(user); // { id: 'u003', name: '次郎' }

9. マジックナンバーは避け、定数・Enumを使おう

コード中に直接数字や文字列を書くのではなく、意味のある名前に置き換えましょう。

// NG 1がなにをあらわす数字あなのか分からない
if (status === 1) {
  // 処理
}

// OK
const STATUS_ACTIVE = 1;
if (status === STATUS_ACTIVE) {
  // 処理
}

牧野

RPCブログ管理者