コーディング原則

コードの読みやすさと品質を担保するための原則。レビュー時の判断基準として使用する。

目的

ESLintやTypeScriptで自動検出できない品質の問題を、実装時・レビュー時に判断するための基準である。

命名

名前だけで意図が伝わるようにする。コードを読む人が、定義を辿らなくても意味を理解できることが基準である。

良い例

// 変数名だけで「セッションが期限切れかどうか」とわかる
const isExpired = session.expiresAt < now;
// 「有効な従業員の一覧」であることが名前から読み取れる
const activeEmployees = employees.filter((e) => e.status === 'active');

悪い例

// 何のflagか不明。定義を読まないと意味がわからない
const flag = session.expiresAt < now;
// 何のlistか不明。employeesのフィルタ結果であることが名前から読み取れない
const list = employees.filter((e) => e.status === 'active');
// 略語で意味が失われている。何の日付かわからない
const d = new Date();

型定義

型定義には type を使用し、interface は使わない。type はすべての型（ユニオン、タプル、プリミティブ等）を表現でき、「どちらを使うか」の判断が不要になる。また、interface の宣言マージによる意図しない型の拡張を防げる。

例外として、Repositoryの契約定義には interface を使用する。依存性逆転のための抽象であり、implements による実装の強制が目的である。

型アサーション（as）

原則として as は使わない。型アサーションはTypeScriptの型チェックを無効化するため、型安全性が失われる。

使うべきでない場合

// Bad: 実行時にundefinedになる可能性をasで握りつぶしている
const employee = employees.find((e) => e.id === id) as Employee;

// Bad: 実際のレスポンスがこの型である保証がない。Valibotでバリデーションすべき
const data = (await response.json()) as Employee;

使ってよい場合

// OK: リテラル型として推論させるための構文であり、型安全性を損なわない
const roles = ['owner', 'manager', 'staff'] as const;

// OK: TypeScriptがコードの文脈から型を絞り込めない場合
// ただし、型ガードやバリデーションで代替できないかを先に検討する
const entries = Object.entries(config) as [keyof Config, string][];

定義と使用箇所の近接

定義はそれを使用する箇所の直前に置く。定義と使用が離れるほど、コードを読む際に前後を行き来する必要が生じ、認知負荷が上がる。

良い例

// Propsはコンポーネントの直前に定義する。コンポーネントを読む時点で型がすぐ見える
type EmployeeCardProps = {
  name: string;
  role: string;
};

const EmployeeCard = ({ name, role }: EmployeeCardProps) => {
  return <div>{name} - {role}</div>;
};

悪い例

// ファイル先頭にすべての型をまとめて定義すると、使用箇所まで距離が離れる
type EmployeeCardProps = { ... };
type EmployeeListProps = { ... };
type EmployeeFormProps = { ... };

// ... 数十行後 ...

const EmployeeCard = ({ name, role }: EmployeeCardProps) => { ... };

説明変数

式の意図が読み取れない場合に、変数に名前をつけて意図を明示する。ただし、式自体が十分に読みやすい場合は不要である。

使うべき場合

変数名が式より多くの情報を伝える場合（複合条件、ビジネスルールの命名）:

// 「承認できるか」というビジネスルールに名前がつき、条件の意図が明確になる
const canApprove = employee.role === 'owner' || employee.role === 'manager';
if (canApprove) {

同じ式が複数回使われる場合:

// 計算結果を変数にすることで、同じ式の重複を避け、意味も明確になる
const overtimeHours = totalHours - standardHours;
const overtimePay = overtimeHours * hourlyRate * 1.25;
const overtimeMessage = `残業${overtimeHours}時間`;

使うべきでない場合

変数名が式の内容をそのまま繰り返しているだけで、情報が増えていない場合:

// Bad: isActiveはemployee.status === 'active'の言い換えでしかなく、情報が増えていない
const isActive = employee.status === 'active';
if (isActive) {

// Good: 式自体が明確なので、変数を挟まない方がシンプルに読める
if (employee.status === 'active') {

インターフェースの分離

クライアントが使わないメソッドやプロパティへの依存を強制しない。型は、そのコードが実際に必要とするフィールドだけを含むようにする。

良い例

// 一覧画面は表示に必要なフィールドだけに依存しており、他のフィールドの変更に影響されない
type EmployeeListItem = {
  id: string;
  name: string;
};

const EmployeeList = ({ employees }: { employees: EmployeeListItem[] }) => {
  return employees.map((e) => <li key={e.id}>{e.name}</li>);
};

悪い例

// 一覧画面で使うのはidとnameだけなのに、全フィールドに依存している
// bankAccountやaddressが変更されたとき、無関係な一覧画面にも影響が及ぶ
type Employee = {
  id: string;
  name: string;
  email: string;
  address: string;
  bankAccount: string;
  emergencyContact: string;
};

const EmployeeList = ({ employees }: { employees: Employee[] }) => {
  return employees.map((e) => <li key={e.id}>{e.name}</li>);
};

ただし、過度に型を分けすぎると管理コストが増える。実際に不要な依存が問題を起こしている場合に適用する。

関数の単一責任

1つの関数は1つのことだけをする。関数名で説明できない処理が含まれていれば、分割を検討する。

分割の判断基準

関数名に「と」や「and」が入る場合、責務が複数ある
関数の中に論理的なブロック（空行で区切られたまとまり）が複数ある場合、それぞれが独立した責務である可能性がある

良い例

// 関数名と処理が一致しており、それぞれの責務が明確である
const validateEmployee = (input: EmployeeInput) => {
  /* 検証のみ */
};
const saveEmployee = (employee: Employee) => {
  /* 保存のみ */
};

悪い例

// 検証・保存・通知の3つの責務が混在しており、変更時に意図しない影響が出やすい
const createEmployee = (input: EmployeeInput) => {
  // 検証...
  // DB保存...
  // メール通知...
};

早期リターン

異常系・境界条件は関数の先頭で処理し、早期にreturnまたはthrowする。不正な状態を検出した時点で即座に処理を中断することで、ネストを浅く保ち、後続の処理に不正なデータが伝播することを防ぐ。

良い例

const getEmployee = (id: string) => {
  const employee = repository.findById(id);
  // 異常系を先に処理することで、以降のコードはemployeeが存在する前提で読める
  if (!employee) {
    throw new NotFoundError('Employee', id);
  }

  return toEmployeeDto(employee);
};

悪い例

const getEmployee = (id: string) => {
  const employee = repository.findById(id);
  // 正常系と異常系がif-elseで同じ階層にあり、主要な処理がネスト内に埋もれている
  if (employee) {
    return toEmployeeDto(employee);
  } else {
    throw new NotFoundError('Employee', id);
  }
};

不変性

変数は原則 const で宣言する。オブジェクトや配列は破壊的変更を避け、新しいオブジェクトを生成する。

良い例

// 新しいオブジェクトを生成するため、元のemployeeは変更されない
const updated = { ...employee, name: newName };
// filterは新しい配列を返すため、元のitemsは変更されない
const filtered = items.filter((item) => item.isActive);

悪い例

// 元のオブジェクトが変更され、他の参照元に予期しない影響を与える
employee.name = newName;
// 元の配列が破壊され、他の処理で同じ配列を参照していた場合にバグになる
items.splice(0, 1);

副作用の分離

データの変換・計算などの純粋な処理と、API呼び出し・DB操作などの副作用を同じ関数に混在させない。

良い例

// 純粋な計算のみ。入力が同じなら常に同じ結果を返すため、テストしやすい
const calculateOvertime = (records: AttendanceRecord[]) => {
  return records.reduce((sum, r) => sum + r.overtime, 0);
};

// 副作用（DBアクセス）を分離し、純粋な計算と組み合わせている
const getMonthlyOvertime = async (employeeId: string, month: string) => {
  const records = await repository.findByMonth(employeeId, month);
  return calculateOvertime(records);
};

DRY

本質的に同じ処理が重複している場合は共通化する。ただし、たまたま似ているだけの処理を無理に共通化しない。

共通化すべき場合

役割が同じ処理が複数箇所に記述されている（一方を変更したとき、他方も変更しなければバグになる）

共通化すべきでない場合

現時点で似ているが、将来的に異なる方向に変化する可能性がある
共通化した結果、条件分岐が増えて複雑になる

YAGNI

現時点で必要のない機能やコードを先回りして書かない。

例

まだ使われていない引数やオプションを関数に追加する
「将来的に必要になるかもしれない」抽象化レイヤーを作る

KISS

コードを必要以上に複雑にしない。同じ結果を達成できるなら、よりシンプルな方法を選ぶ。

悪い例

三項演算子で真偽値を返す（比較式自体が真偽値なので、三項演算子が冗長である）:

// Bad
const isAdult = age >= 18 ? true : false;

// Good
const isAdult = age >= 18;

ネストした三項演算子（条件が増えるたびに読みにくくなり、分岐の全体像が把握しづらい）:

// Bad
const label =
  role === 'owner'
    ? 'オーナー'
    : role === 'manager'
      ? 'マネージャー'
      : 'スタッフ';

// Good
const roleLabels = {
  owner: 'オーナー',
  manager: 'マネージャー',
  staff: 'スタッフ',
} as const;
const label = roleLabels[role];

早すぎる抽象化（propsが増えて複雑化し、個別の変更が全体に影響するようになる）:

// Bad: 2つのページで似たフォームがあるだけで共通コンポーネントを作る
const GenericForm = ({ fields, onSubmit, validate, transform, ... }) => { ... };

// Good: それぞれのページで素直に書く。本当に重複が問題になった時点で共通化する

複雑さの兆候

1つの式にロジックが詰め込まれていて、読むのに時間がかかる
ジェネリクスやユーティリティ型が深くネストしている
抽象化が増えた結果、処理の流れを追うのにファイルを何個も開く必要がある

コメントなしでも読めるコードを目指す。コメントを書く場合は、処理の説明ではなく、その実装を選んだ理由を書く。

書くべきコメント

// NOTE: Better Authのセッションテーブルに外部キー制約があるため、
// 従業員削除時にセッションを先に削除する必要がある
// 削除順の理由がコードからは読み取れないため、コメントで補足している
await sessionRepository.deleteByUserId(userId);
await employeeRepository.delete(employeeId);

書くべきでないコメント

// 従業員を取得する — コードを読めばわかることを繰り返しているだけ
const employee = repository.findById(id);

// nullチェック — 何をしているかではなく、なぜそうするかが重要
if (!employee) {
  throw new NotFoundError('Employee', id);
}

ファイル分割

ファイルの中身を一言で説明できることを基準とする。

分割すべき場合

責務が明確に異なるコードが1ファイルに混在している
ファイルが長すぎて見通しが悪い
他の場所から再利用したい単位がある

分割すべきでない場合

凝集度が高く、密接に関連するコードである
分割した結果、ファイル間の相互参照が増える
分割した結果、数十行しかないファイルが生まれる

exportの範囲

外部から使われるものだけをexportする。exportされたものは他のファイルから依存されるため、変更時の影響範囲が広がる。exportしなければ、ファイル内で自由にリファクタできる。

exportする

他のファイルから呼び出される関数・型・定数

exportしない

そのファイル内でしか使わないヘルパー関数・内部定数

悪い例

// 将来使うかもしれないからexportする — YAGNIに反する。必要になった時点でexportすればよい
export const internalHelper = () => { ... };

// テストのためだけにexportする — 内部実装のテストになり、リファクタでテストが壊れる原因になる
export const _parseInput = (raw: string) => { ... };

コーディング原則

On this page