コード品質

概要

保守性に優れ、読みやすく、信頼できるコードを書くための原則と実践。

---

クリーンコード原則

意味のある名前

// ❌ 謎めいた名前
const d = new Date();
const u = getU();
const arr = data.filter(x => x.s === 'a');

// ✅ 説明的な名前
const currentDate = new Date();
const currentUser = getCurrentUser();
const activeUsers = users.filter(user => user.status === 'active');

// ❌ ハンガリアン記法 (廃止予定)
const strName = 'John';
const arrItems = [];
const bIsActive = true;

// ✅ 型システムに型処理を任せる
const name = 'John';
const items: Item[] = [];
const isActive = true;

関数

// ❌ やることが多すぎる
function processUserData(userId: string) {
  const user = db.findUser(userId);
  const orders = db.findOrders(userId);
  const total = orders.reduce((sum, o) => sum + o.amount, 0);
  sendEmail(user.email, `Your total: ${total}`);
  updateAnalytics(userId, total);
  return { user, orders, total };
}

// ✅ 単一責任
function getUser(userId: string): User {
  return db.findUser(userId);
}

function getUserOrders(userId: string): Order[] {
  return db.findOrders(userId);
}

function calculateTotal(orders: Order[]): number {
  return orders.reduce((sum, o) => sum + o.amount, 0);
}

function sendOrderSummary(user: User, total: number): void {
  sendEmail(user.email, `Your total: ${total}`);
}

// ❌ パラメータが多すぎる
function createUser(name, email, age, role, department, manager, startDate) {}

// ✅ オブジェクトパラメータを使用
interface CreateUserParams {
  name: string;
  email: string;
  age?: number;
  role: Role;
  department: string;
  managerId?: string;
  startDate: Date;
}

function createUser(params: CreateUserParams): User {}

コメント

// ❌ 冗長なコメント
// カウンターを1増やす
counter++;

// ❌ 古いコメント (コードは変わったがコメントは変わらず)
// ユーザーのメールアドレスを返す
function getUserEmail(user: User) {
  return user.email;
}

// ✅ WHY を説明する (WHAT ではなく)
// ソート済みリストで 100k+ アイテムが存在する可能性があるため、二分探索を使用
const index = binarySearch(sortedItems, target);

// ✅ 非自明な動作について警告
// 重要: この関数はパフォーマンスのため入力配列を変更します
function quickSort(arr: number[]): number[] {
  // ...
}

// ✅ コンテキスト付き TODO
// TODO(john): マイグレーション完了後に削除 - JIRA-1234 で追跡中
const legacyAdapter = new LegacyAdapter();

---

SOLID 原則

単一責任原則

// ❌ 複数の責任
class UserManager {
  createUser(data: UserData) { /* DB ロジック */ }
  validateEmail(email: string) { /* 検証ロジック */ }
  sendWelcomeEmail(user: User) { /* メールロジック */ }
  generateReport(users: User[]) { /* レポートロジック */ }
}

// ✅ 各クラスが単一責任
class UserRepository {
  create(data: UserData): User { /* DB ロジック */ }
  findById(id: string): User | null { /* DB ロジック */ }
}

class UserValidator {
  validateEmail(email: string): boolean { /* 検証 */ }
  validatePassword(password: string): ValidationResult { /* 検証 */ }
}

class EmailService {
  sendWelcomeEmail(user: User): void { /* メールロジック */ }
}

class UserReportGenerator {
  generate(users: User[]): Report { /* レポートロジック */ }
}

開放-閉鎖原則

// ❌ 新しい支払い方法を追加するには修正が必要
class PaymentProcessor {
  process(payment: Payment) {
    if (payment.type === 'credit') {
      // クレジットカードロジック
    } else if (payment.type === 'paypal') {
      // PayPal ロジック
    } else if (payment.type === 'crypto') {
      // 暗号通貨ロジック - 既存コードを修正した！
    }
  }
}

// ✅ 拡張に開放、修正に閉鎖
interface PaymentMethod {
  process(amount: number): Promise<PaymentResult>;
}

class CreditCardPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> { /* ... */ }
}

class PayPalPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> { /* ... */ }
}

// 新しい支払い方法 - 既存コードの修正不要
class CryptoPayment implements PaymentMethod {
  async process(amount: number): Promise<PaymentResult> { /* ... */ }
}

class PaymentProcessor {
  constructor(private method: PaymentMethod) {}

  async process(amount: number): Promise<PaymentResult> {
    return this.method.process(amount);
  }
}

リスコフの置換原則

// ❌ LSP 違反 - Square が Rectangle の契約を破る
class Rectangle {
  constructor(public width: number, public height: number) {}

  setWidth(w: number) { this.width = w; }
  setHeight(h: number) { this.height = h; }
  getArea() { return this.width * this.height; }
}

class Square extends Rectangle {
  setWidth(w: number) {
    this.width = w;
    this.height = w; // 予期しない副作用！
  }
  setHeight(h: number) {
    this.width = h;
    this.height = h; // 予期しない副作用！
  }
}

// ✅ 適切な抽象化
interface Shape {
  getArea(): number;
}

class Rectangle implements Shape {
  constructor(private width: number, private height: number) {}
  getArea() { return this.width * this.height; }
}

class Square implements Shape {
  constructor(private side: number) {}
  getArea() { return this.side * this.side; }
}

インターフェース分離原則

// ❌ 肥大化したインターフェース
interface Worker {
  work(): void;
  eat(): void;
  sleep(): void;
  attendMeeting(): void;
  writeReport(): void;
}

// ロボットは食べたり寝たりできない！
class Robot implements Worker {
  work() { /* ... */ }
  eat() { throw new Error('Robots do not eat'); }  // 実装を強要される
  sleep() { throw new Error('Robots do not sleep'); }
  // ...
}

// ✅ 分離されたインターフェース
interface Workable {
  work(): void;
}

interface Eatable {
  eat(): void;
}

interface Sleepable {
  sleep(): void;
}

class Human implements Workable, Eatable, Sleepable {
  work() { /* ... */ }
  eat() { /* ... */ }
  sleep() { /* ... */ }
}

class Robot implements Workable {
  work() { /* ... */ }
}

依存性逆転原則

// ❌ 高レベルが低レベルに依存
class OrderService {
  private db = new MySQLDatabase();  // 具体的な依存
  private mailer = new SendGridMailer();  // 具体的な依存

  createOrder(data: OrderData) {
    const order = this.db.insert('orders', data);
    this.mailer.send(data.email, 'Order confirmed');
    return order;
  }
}

// ✅ 抽象に依存
interface Database {
  insert(table: string, data: unknown): unknown;
  find(table: string, query: unknown): unknown[];
}

interface Mailer {
  send(to: string, message: string): void;
}

class OrderService {
  constructor(
    private db: Database,
    private mailer: Mailer
  ) {}

  createOrder(data: OrderData) {
    const order = this.db.insert('orders', data);
    this.mailer.send(data.email, 'Order confirmed');
    return order;
  }
}

// これでどんな実装でも注入可能
const service = new OrderService(
  new PostgresDatabase(),
  new SESMailer()
);

---

コードレビューのベストプラクティス

チェック項目

## コードレビューチェックリスト

### 正確性
- [ ] ロジックが正確で、エッジケースを処理している
- [ ] エラーハンドリングが適切
- [ ] 明らかなバグや回帰がない

### 設計
- [ ] コードが適切な抽象レベルにある
- [ ] 不要な複雑さがない
- [ ] コードベース内の既存パターンに従っている

### 可読性
- [ ] 明確な命名と意図
- [ ] コメントは "何" ではなく "なぜ" を説明している
- [ ] 可能な限りコードが自己説明的

### テスト
- [ ] 適切なテストカバレッジ
- [ ] テストが有意味 (カバレッジの水増しではない)
- [ ] エッジケースがテストされている

### パフォーマンス
- [ ] 明らかな N+1 クエリや非効率がない
- [ ] 適切なデータ構造が使用されている
- [ ] 必要に応じてキャッシングを検討している

### セキュリティ
- [ ] 入力検証が存在する
- [ ] コード内にシークレットがない
- [ ] 認証・認可が正しい

フィードバックを与える

# 良いレビューコメント

## ✅ 具体的で実行可能
"このループは O(n²) の計算量があります。O(n) ルックアップのために Map を使用することを検討してください。"

## ✅ なぜかを説明する
"これを別の関数に抽出しましょう - ロジックをテストしやすくでき、
メイン関数がより読みやすくなります。"

## ✅ 代替案を提示する
"配列を変更する代わりに、`filter()` を使用して新しい配列を返すことを検討してください: `const active = items.filter(i => i.active)`"

## ✅ 深刻度を区別する
- "nit: " - 軽微なスタイル問題、オプション
- "suggestion: " - 良いことだが、ブロッキングではない
- "blocking: " - マージ前に修正が必須

# 避けるべきこと

## ❌ 曖昧な批判
"このコードは混乱している"

## ❌ 人格攻撃
"あなたはいつもこのミスをします"

## ❌ 説明なし
"別のアプローチを使用してください"

---

コードメトリクス

循環的複雑度

// 複雑度が高い (10+) - テストと保守が難しい
function processOrder(order: Order): Result {
  if (order.status === 'pending') {           // +1
    if (order.paymentMethod === 'card') {     // +1
      if (order.amount > 1000) {              // +1
        // ...
      } else if (order.amount > 100) {        // +1
        // ...
      } else {
        // ...
      }
    } else if (order.paymentMethod === 'cash') {  // +1
      // ...
    }
  } else if (order.status === 'processing') {  // +1
    // ...
  }
  // ... さらに分岐
}

// 複雑度が低い - 条件を抽出
function processOrder(order: Order): Result {
  const processor = getProcessor(order.paymentMethod);
  const tier = getPricingTier(order.amount);
  return processor.process(order, tier);
}

追跡すべきメトリクス

メトリクス	目標値	理由
循環的複雑度	< 10 (関数ごと)	テスト可能性
関数の長さ	< 50 行	可読性
ファイルの長さ	< 400 行	保守性
テストカバレッジ	> 80%	信頼度
重複	< 3%	DRY 原則

---

Lint と フォーマット

ESLint 設定

// .eslintrc.js
module.exports = {
  extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended',
  ],
  rules: {
    // バグを防ぐ
    'no-unused-vars': 'error',
    '@typescript-eslint/no-floating-promises': 'error',
    '@typescript-eslint/no-misused-promises': 'error',

    // コード品質
    'complexity': ['warn', 10],
    'max-lines-per-function': ['warn', 50],
    'max-depth': ['warn', 3],

    // 一貫性
    'prefer-const': 'error',
    'no-var': 'error',
  }
};

Pre-commit フック

// package.json
{
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.{ts,tsx}": [
      "eslint --fix",
      "prettier --write"
    ],
    "*.{json,md}": [
      "prettier --write"
    ]
  }
}

---

関連スキル

• [[refactoring]] - 既存コードの改善
• [[testing-strategies]] - テストを通じた品質保証
• [[design-patterns]] - 実証済みのソリューション

---

危険な落とし穴 (常見陷阱)

これらはコード品質における最も一般的でコストが高い誤り

SE-1: 過度なエンジニアリング (Over-engineering)

• 重大度: high
• 状況: 「将来的に必要かもしれない」という理由で不要な抽象化と複雑さを追加
• 原因: YAGNI 原則の無視、デザインパターンの乱用、「もし後で...だったら」という考え方
• 症状:
  • シンプルな機能の変更に 10+ ファイルの修正が必要
  • 新人が架構を理解できない
  • コードが要件より 10 倍複雑
• 検出: Factory.*Factory|Abstract.*Abstract|interface.*\{.*\}(?=.*interface.*\{.*\})|Strategy.*Strategy
• 解法: YAGNI (You Aren't Gonna Need It)、最もシンプルな実装から始める、必要になったらリファクタリング

SE-2: 命名の不一貫性

• 重大度: medium
• 状況: 同じコンセプトが異なる場所で異なる名前を持つ、または異なるコンセプトが似た名前を持つ
• 原因: 統一された用語がない、複数人開発での調整不足、コピペ時の名前変更漏れ
• 症状:
  • user, customer, client, account が同じものを指している
  • 関連コードの検索ができない
  • 新人が「これとあれの違いは？」とよく質問する
• 検出: (user|customer|client|account).*=.*find|(get|fetch|retrieve|load).*User
• 解法: 用語集 (Ubiquitous Language) の作成、コードレビュー時の命名チェック、命名の統一化リファクタリング

SE-3: 深いネスト (Deep Nesting)

• 重大度: medium
• 状況: if-else やコールバックのネストが 3-4 レベルを超え、読みにくくなっている
• 原因: 早期リターンの欠如、関数の抽出不足、コールバック地獄
• 症状:
  • コードを見るために横方向スクロールが必要
  • どの } がどの { に対応するか追いにくい
  • 循環的複雑度が非常に高い
• 検出: \{.*\{.*\{.*\{|if.*if.*if.*if|\.then\(.*\.then\(.*\.then\(
• 解法: Guard clause (早期リターン)、関数抽出、Promise/async-await によるコールバック回避

SE-4: マジックナンバー/文字列 (Magic Numbers)

• 重大度: medium
• 状況: コード内で意味不明の数字や文字列が直接使われている
• 原因: 定数定義の手抜き、「1 回だけだから抽出不要」という考え
• 症状:
  • 86400 を見ても何か分からない (1 日の秒数)
  • 修正時にグローバル置換が必要
  • 異なる場所で同じ値が異なる意味を持つ
• 検出: \b(86400|3600|1000|60000|1024|65535)\b|status\s*===?\s*['"][^'"]+['"]
• 解法: 意味のある定数名に抽出、enum を使用、設定値の一元管理

SE-5: 大泥球 (Big Ball of Mud)

• 重大度: critical
• 状況: コードに明確なアーキテクチャがなく、すべてが混在し、至る所に依存がある
• 原因: モジュール化設計の欠如、期限に追われて「とりあえず動かす」、リファクタリング不足
• 症状:
  • A を修正すると B が壊れる
  • 単一ファイルが 1000+ 行
  • すべてがすべてをインポートしている
• 検出: import.*from.*\.\.\/\.\.\/\.\.\/|require\(.*\.\..*\.\..*\.\.\)|lines.*>\s*1000
• 解法: 層別アーキテクチャ、明確なモジュール境界、定期的なリファクタリング、厳格な import ルール

---

検証

V-1: console.log の禁止 (本番コード)

• 種類: regex
• 重大度: medium
• パターン: console\.(log|debug|info)\(
• メッセージ: console.log は本番コードに含めるべきではありません
• 修復提案: 適切なロガーを使用してください (winston, pino など) またはデバッグステートメントを削除してください
• 適用対象: *.ts, *.js

V-2: any 型の禁止

• 種類: ast
• 重大度: high
• パターン: TSAnyKeyword
• メッセージ: 'any' 型は TypeScript の型安全性を損なわせます
• 修復提案: 具体的な型、'unknown'、またはジェネリック型パラメータを使用してください
• 適用対象: *.ts, *.tsx

V-3: 関数パラメータ数が多すぎる

• 種類: regex
• 重大度: medium
• パターン: function\s+\w+\s*\([^)]*,\s*[^)]*,\s*[^)]*,\s*[^)]*,\s*[^)]*\)|=>\s*\([^)]*,\s*[^)]*,\s*[^)]*,\s*[^)]*,\s*[^)]*\)
• メッセージ: 関数に 4 個以上のパラメータがあります - オブジェクトの使用を検討してください
• 修復提案: 複数のパラメータを単一のオプションオブジェクトに置き換えてください: function(options: Options)
• 適用対象: *.ts, *.js

V-4: 追跡なしの TODO

• 種類: regex
• 重大度: low
• パターン: //\s*TODO(?!.*#\d|.*JIRA|.*\w+-\d+)
• メッセージ: TODO コメントに追跡参照がありません
• 修復提案: Issue 参照を追加してください: // TODO(#123): description またはチケットを作成してください
• 適用対象: *.ts, *.js, *.tsx, *.jsx

V-5: 深い相対パスでのインポート

• 種類: regex
• 重大度: medium
• パターン: import.*from\s+['"]\.\.\/\.\.\/\.\.\/|require\s*\(\s*['"]\.\.\/\.\.\/\.\.\/
• メッセージ: 深い相対インポートはモジュール境界が不適切であることを示唆しています
• 修復提案: パスエイリアスを使用してください: import { X } from '@/modules/x'
• 適用対象: *.ts, *.js, *.tsx, *.jsx