20
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

SchooAdvent Calendar 2024

Day 15

コードレビュー時に意識したい7つの設計原理

Last updated at Posted at 2024-12-14

はじめに

この記事はSchoo Advent Calendar2024の15日目の記事です!

株式会社Schooに所属しています、新卒2年目のエンジニアの小磯です:santa_tone1:
この記事では、エンジニアの業務において欠かせない「コードレビュー」を行う際にも参考になる書籍「プリンシプル オブ プログラミング」から7つの原則を紹介させていただきます。

私自身、入社して実務に入って以来、先輩エンジニアの方にコードレビューをしていただく機会がたくさんありました。
様々な観点からレビューをいただく中で、優れたエンジニアはレビュー時にどのような観点で見ているのか漏れやブレがなく一貫性のある指摘を行うためにどのようなことを意識しているのか、悩んだ際にとても参考になったので改めてまとめてみました。

コードレビューをする側やエンジニアだけに限らず、複雑な仕事や資料の整理でも活用されるMECEフレームワークにも通ずるような観点ですので、ぜひお読みいただけると幸いです!

7つの設計原理とは

いきなりですが、以下が本記事で紹介する7つの設計原理になります。

  1. 単純原理(Simplicity Principle)
  2. 同型原理(Isomorphism Principle)
  3. 対称原理(Symmetry Principle)
  4. 階層原理(Hierarchy Principle)
  5. 線形原理(Linearity Principle)
  6. 明証原理(Clarity Principle)
  7. 安全原理(Safety Principle)

どれも実際のソフトウェア開発現場の分析から導出された、品質の良いコードのための「経験則」です。
具体的なコードも交えつつ以降で詳しく紹介させていただきます。
(サンプルコードはPHPのバージョン5.4以降で動かすことを想定しています)

1. 単純原理(Simplicity Principle)

単純原理とは、「シンプルにこだわる」ということです。
極論、プログラミングの初心者でも読めるよう、一貫して単純なコードを書くようにします。
私自身、スキルがない時ほど、シンプルに書くことに難しさを感じ、「この記述は不要」や「この書き方の方がより簡潔」と言った指摘を今までたくさん受けてきました。

OKコード例
<?php

// 要件: 配列内の数値の合計を求め、結果を返す

function calculateSum(array $numbers): int {
    return array_sum($numbers);
}

// 使用例
$numbers = [1, 2, 3, 4, 5];
echo '合計: ' . calculateSum($numbers) . PHP_EOL; // 出力: 合計: 15

良い点:

  • 明確な意図:array_sum 関数を使用しており、「配列の合計を計算する」という意図が一目で分かる
  • 短く読みやすい:冗長な処理がなく、1行で目的を達成している
  • 再利用可能:関数にすることで、他の場面でも同じ処理を簡単に利用できる
NGコード例
<?php

// 要件: 配列内の数値の合計を求め、結果を返す

function calculateSum(array $numbers): int {
    $sum = 0;
    foreach ($numbers as $number) {
        $sum += $number;
    }
    return $sum;
}

// 使用例
$numbers = [1, 2, 3, 4, 5];
echo '合計: ' . calculateSum($numbers) . PHP_EOL; // 出力: 合計: 15

悪い点:

  • 冗長な記述:foreach を使うことで、簡単に実現可能な処理を必要以上に複雑に記述している
  • メンテナンス性が低い:ロジックが明示的すぎて変更や追加の際にコードを再構築する可能性が高い
  • 標準関数の活用不足:PHPには array_sum という専用の標準関数があるため、不要なコードを増やしている

2. 同型原理(Isomorphism Principle)

同型原理とは、「形にこだわる」という原理です。
同じことが同じように表現されていると、逆に「違うもの」が際立ちます。違うものとはつまり、正しくない=バグの原因となりやすい部分です。
コード上の違和感に気付き、障害を見つけやすくするためにも、一貫性のあるコードを書く必要があります。

OKコード例
<?php

// 要件: 配列内の偶数と奇数をそれぞれ集計する関数を実装する

function countNumbers(array $numbers, callable $condition): int {
    return count(array_filter($numbers, $condition));
}

function countEvenNumbers(array $numbers): int {
    return countNumbers($numbers, fn($n) => $n % 2 === 0);
}

function countOddNumbers(array $numbers): int {
    return countNumbers($numbers, fn($n) => $n % 2 !== 0);
}

// 使用例
$numbers = [1, 2, 3, 4, 5, 6];
echo '偶数の数: ' . countEvenNumbers($numbers) . PHP_EOL; // 出力: 偶数の数: 3
echo '奇数の数: ' . countOddNumbers($numbers) . PHP_EOL; // 出力: 奇数の数: 3

良い点:

  • 構造が一貫している:共通のロジック( countNumbers )を抽象化しているため、処理方法が統一されている
  • 再利用性が高い: countNumbers は条件を変えれば、偶数・奇数以外の条件にも簡単に対応できる
  • 可読性が高い:関数名と処理内容が直感的に一致しており、新しい開発者にも分かりやすい
NGコード例
<?php

// 要件: 配列内の偶数と奇数をそれぞれ集計する関数を実装する

function countEvenNumbers(array $numbers): int {
    $count = 0;
    foreach ($numbers as $number) {
        if ($number % 2 === 0) {
            $count++;
        }
    }
    return $count;
}

function countOddNumbers(array $numbers): int {
    return count(array_filter($numbers, fn($n) => $n % 2 !== 0));
}

// 使用例
$numbers = [1, 2, 3, 4, 5, 6];
echo '偶数の数: ' . countEvenNumbers($numbers) . PHP_EOL; // 出力: 偶数の数: 3
echo '奇数の数: ' . countOddNumbers($numbers) . PHP_EOL; // 出力: 奇数の数: 3

悪い点:

  • 一貫性がない:偶数の処理では foreach を使用しているのに対し、奇数の処理では array_filter を使用しており、方法が異なる
  • 保守性が低い:処理方法が異なるため、要件が変更された場合に修正箇所が増える可能性が高い
  • 可読性が低い:「なぜ偶数と奇数で処理が異なるのか」を明確に説明できず、意図が不明瞭

3. 対称原理(Symmetry Principle)

対称原理とは、「形の対称性にこだわる」という原理です。
上があれば下、左があれば右、アクティブがあればパッシブというような、対称性にこだわります。
コードの形に対称性があると、予測がつきやすくなるため、コード理解のスピードが段違いに早くなるなと実感しているのと、設計の考慮漏れを防ぐことにも繋がっていきます。

OKコード例
<?php

// 要件: ユーザーの役割(管理者または一般ユーザー)に基づき、アクセスを制御する

class User {
    private string $role;

    public function __construct(string $role) {
        $this->role = $role;
    }

    public function isAdmin(): bool {
        return $this->role === 'admin';
    }

    public function isUser(): bool {
        return $this->role === 'user';
    }
}

// 使用例
$user = new User('admin');

if ($user->isAdmin()) {
    echo '管理者向けのページを表示します。' . PHP_EOL;
} elseif ($user->isUser()) {
    echo '一般ユーザー向けのページを表示します。' . PHP_EOL;
} else {
    echo 'アクセス権がありません。' . PHP_EOL;
}

良い点:

  • 対称的な命名:isAdminisUser のメソッド名が対称的で、一貫性がある
  • 予測可能性が高い:他のロールが追加される場合も、類似のメソッド(例: isEditor )が容易に追加可能
  • 拡張性が高い:ロールの種類が増えても、同じパターンで実装できる
NGコード例
<?php

// 要件: ユーザーの役割(管理者または一般ユーザー)に基づき、アクセスを制御する

class User {
    public string $role;

    public function __construct(string $role) {
        $this->role = $role;
    }

    public function isAdmin(): bool {
        return $this->role === 'admin';
    }

    // 一般ユーザーのチェックは直接文字列を比較
}

// 使用例
$user = new User('user');

if ($user->isAdmin()) {
    echo '管理者向けのページを表示します。' . PHP_EOL;
} elseif ($user->role === 'user') { // 直接プロパティを参照
    echo '一般ユーザー向けのページを表示します。' . PHP_EOL;
} else {
    echo 'アクセス権がありません。' . PHP_EOL;
}

悪い点:

  • 不統一な構造:管理者チェックにはメソッドを使用し、一般ユーザーには直接プロパティを参照している
  • 保守性が低い:プロパティを不必要に公開しており、role プロパティの仕様が変更された場合、プロパティを直接参照している箇所すべてを修正する必要がある
  • 予測可能性が低い:新しいロールが追加されると、同様の不統一なロジックがさらに増える可能性が高い

4. 階層原理(Hierarchy Principle)

階層原理とは、「構造が階層であることにこだわる」という原理です。
物事の主従関係、前後関係など、階層構造を常に意識し、整理された関係性を構築します。注意しないと異なる階層を行ったり来たりしてしまうコードを書いてしまいがちでしたが、階層ごとに処理を取り決め、同じ種類の処理が異なる階層間に渡らないようにすることが重要になります。

OKコード例
<?php

// 要件: ユーザーの情報を取得し、ビューで表示する

// データ層
class UserRepository {
    public function getUserById(int $id): array {
        // 仮データとしてユーザー情報を返す
        return [
            'id' => $id,
            'name' => 'Taro Yamada',
            'email' => 'taro@example.com',
        ];
    }
}

// ビジネスロジック層
class UserService {
    private UserRepository $repository;

    public function __construct(UserRepository $repository) {
        $this->repository = $repository;
    }

    public function getUserDetails(int $id): array {
        return $this->repository->getUserById($id);
    }
}

// プレゼンテーション層
class UserController {
    private UserService $service;

    public function __construct(UserService $service) {
        $this->service = $service;
    }

    public function showUser(int $id): void {
        $user = $this->service->getUserDetails($id);
        echo 'ユーザー名: ' . $user['name'] . PHP_EOL;
        echo 'メールアドレス: ' . $user['email'] . PHP_EOL;
    }
}

// 使用例
$repository = new UserRepository();
$service = new UserService($repository);
$controller = new UserController($service);

$controller->showUser(1);

良い点:

  • 責務の分離:データ取得はリポジトリ、ビジネスロジックはサービス、プレゼンテーションはコントローラーと、抽象レベルを意識した階層構造で作られている
  • 再利用性:リポジトリやサービスは他の部分でも再利用できる
  • テストの容易さ:各層が独立しているため、単体テストが容易に行える
  • 変更に強い:データベースの変更や表示フォーマットの変更が、それぞれの層で完結する
NGコード例
<?php

// 要件: ユーザーの情報を取得し、ビューで表示する

class UserController {
    public function showUser(int $id): void {
        // データ取得とビジネスロジックがコントローラー内に混在
        $user = [
            'id' => $id,
            'name' => 'Taro Yamada',
            'email' => 'taro@example.com',
        ];

        // プレゼンテーション
        echo 'ユーザー名: ' . $user['name'] . PHP_EOL;
        echo 'メールアドレス: ' . $user['email'] . PHP_EOL;
    }
}

// 使用例
$controller = new UserController();
$controller->showUser(1);

悪い点:

  • 責務の集中:コントローラーがデータ取得、ビジネスロジック、表示ロジックすべてを担い、責任が集中している
  • 再利用性が低い:データ取得やロジックがコントローラー内にハードコーディングされており、他で再利用できない
  • 変更に弱い:仕様変更があると、コントローラー全体を修正する必要があり、影響範囲が広がる
  • テストが困難:コントローラー内でデータ取得や表示ロジックを混ぜているため、単体テストが難しい

5. 線形原理(Linearity Principle)

線形原理とは、「処理の流れは直線であることにこだわる」という原理です。
階層の上位から下位に向かって流れていくような、全体を通じて一貫性のある処理の流れにすると、可読性が格段に向上します。反対に、スイッチでコードを制御したり、状態の数を無闇に増やしたりすると、コードの見通しが悪くなるばかりでなく、障害の温床にもなります。

OKコード例
<?php

// 要件: ユーザー入力を検証し、成功時にはデータベースに保存し、失敗時にはエラーを表示する

class UserValidator {
    public function validate(array $data): bool {
        if (empty($data['name']) || empty($data['email'])) {
            return false;
        }
        return true;
    }
}

class UserRepository {
    public function save(array $data): bool {
        // データベースに保存 (仮実装)
        return true;
    }
}

class UserController {
    private UserValidator $validator;
    private UserRepository $repository;

    public function __construct(UserValidator $validator, UserRepository $repository) {
        $this->validator = $validator;
        $this->repository = $repository;
    }

    public function createUser(array $data): void {
        if (!$this->validator->validate($data)) {
            echo 'エラー: 入力が無効です。' . PHP_EOL;
            return;
        }

        if (!$this->repository->save($data)) {
            echo 'エラー: データ保存に失敗しました。' . PHP_EOL;
            return;
        }

        echo '成功: ユーザーが保存されました。' . PHP_EOL;
    }
}

// 使用例
$validator = new UserValidator();
$repository = new UserRepository();
$controller = new UserController($validator, $repository);

$data = ['name' => 'Taro', 'email' => 'taro@example.com'];
$controller->createUser($data);

良い点:

  • 単一方向の流れ:処理が「検証 → 保存 → 成功/失敗」と一貫して順序立てて進む
  • 早期リターン:エラー時に早期リターンすることで不要な処理を避け、コードの可読性を向上
  • 責務の分離:検証・データ保存・結果出力が別々のクラスに分かれており、責任が明確
NGコード例
<?php

// 要件: ユーザー入力を検証し、成功時にはデータベースに保存し、失敗時にはエラーを表示する

class UserController {
    public function createUser(array $data): void {
        $valid = true;

        if (empty($data['name'])) {
            echo 'エラー: 名前が空です。' . PHP_EOL;
            $valid = false;
        }

        if (empty($data['email'])) {
            echo 'エラー: メールアドレスが空です。' . PHP_EOL;
            $valid = false;
        }

        if ($valid) {
            // データベース保存 (仮実装)
            $saved = true;

            if ($saved) {
                echo '成功: ユーザーが保存されました。' . PHP_EOL;
            } else {
                echo 'エラー: データ保存に失敗しました。' . PHP_EOL;
            }
        } else {
            echo 'エラー: 入力が無効です。' . PHP_EOL;
        }
    }
}

// 使用例
$controller = new UserController();
$data = ['name' => 'Taro', 'email' => ''];
$controller->createUser($data);

悪い点:

  • 分岐が多い:フラグ変数 $valid による状態管理が複雑で、コードの流れがわかりにくい
  • ネストが深い:if の入れ子が増え、処理の追跡が難しい
  • 冗長なロジック:エラー時に複数箇所でメッセージを出力しており、コードが冗長

6. 明証原理(Clarity Principle)

明証原理とは、「ロジックの明証性にこだわる」ということです。
明証性とは、はっきりと証明することを指します。つまり、コードは一目見て意図が明らかであり、読み手が理解しやすいように書くことが求められます。
コード上で表現しきれない部分があれば、コメントを残したり、別途ドキュメントを作成したりすることで、より読み手に自分の意図を伝えられるようになりました。

OKコード例
<?php

// 要件: ユーザーの年齢を基に成人かどうかを判定する

class User {
    private int $age;

    public function __construct(int $age) {
        $this->age = $age;
    }

    public function isAdult(): bool {
        return $this->age >= 18;
    }
}

$user = new User(20);
if ($user->isAdult()) {
    echo 'このユーザーは成人です。' . PHP_EOL;
} else {
    echo 'このユーザーは未成年です。' . PHP_EOL;
}

良い点:

  • 自己説明的な名前付け:メソッド名 isAdult は、成人かどうかを判定することが明確にわかる
  • 適切な抽象化:年齢判定ロジックが User クラス内にあり、コードの意図が簡単に理解できる
  • 直感的な構造:読むだけで何をしているかすぐに理解可能
NGコード例
<?php

// 要件: ユーザーの年齢を基に成人かどうかを判定する

class User {
    private int $a; // 年齢だが、変数名が不明瞭

    public function __construct(int $a) {
        $this->a = $a;
    }

    public function check(): bool {
        return $this->a >= 18; // 何をチェックしているか不明
    }
}

$user = new User(20);
if ($user->check()) {
    echo 'OK' . PHP_EOL; // 意味が曖昧
} else {
    echo 'NG' . PHP_EOL; // 意味が曖昧
}

悪い点:

  • 名前が不明瞭:変数名 $a やメソッド名 check が何を意味しているか分からない
  • 曖昧な出力:"OK" や "NG" では何が成功/失敗なのかが不明
  • 意図がわかりにくい:コードを読んでも、何が成人判定なのか直感的に理解できない

7. 安全原理(Safety Principle)

安全原理とは、「安全性にこだわる」ということです。
起こりうる様々なパターンの想定をして、それぞれ安全な方向に動作するように設計します。例えば、ある if 文に対して else 文の考慮、ある case 文に対して default 句の考慮、ある変数に対しての NULL チェックなどは最低限考慮し、+αの部分はその都度考えるようにしています。

OKコード例
<?php

// 要件: ユーザーのデータを受け取り、名前とメールアドレスを保存する

class User {
    private string $name;
    private string $email;

    public function __construct(string $name, string $email) {
        if (empty($name)) {
            throw new InvalidArgumentException('名前は空にできません。');
        }
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
            throw new InvalidArgumentException('有効なメールアドレスを指定してください。');
        }
        $this->name = $name;
        $this->email = $email;
    }

    public function save(): bool {
        try {
            // データベースに保存する処理(仮実装)
            return true; // 保存成功
        } catch (Exception $e) {
            error_log('保存に失敗: ' . $e->getMessage());
            return false; // 保存失敗
        }
    }
}

// 使用例
try {
    $user = new User('Taro', 'taro@example.com');
    if ($user->save()) {
        echo '保存が成功しました。' . PHP_EOL;
    } else {
        echo '保存に失敗しました。' . PHP_EOL;
    }
} catch (InvalidArgumentException $e) {
    echo 'エラー: ' . $e->getMessage() . PHP_EOL;
}

良い点:

  • 事前条件の検証:コンストラクタで入力データの妥当性をチェックし、不正なデータを受け付けない
  • 例外処理の導入:データベース保存中のエラーをキャッチし、適切にログを記録
  • エラーの明示:無効な入力やエラー状況を詳細に通知し、安全に対処
NGコード例
<?php

// 要件: ユーザーのデータを受け取り、名前とメールアドレスを保存する

class User {
    private $name; // 型指定なし
    private $email;

    public function __construct($name, $email) {
        $this->name = $name;
        $this->email = $email;
    }

    public function save() {
        // データベース保存処理(仮実装)
        // 例外処理なし
        return true;
    }
}

// 使用例
$user = new User('', 'invalid-email');
if ($user->save()) {
    echo '保存が成功しました。' . PHP_EOL;
} else {
    echo '保存に失敗しました。' . PHP_EOL;
}

悪い点:

  • 入力検証の欠如:name が空でも email が無効でもエラーが発生しない
  • 例外処理がない:保存処理中のエラーがシステム全体に影響を与える可能性がある
  • 型安全性が低い:型指定がないため、意図しないデータ型のエラーが起こりうる

最後に

以上、コードレビュー時に意識したい7つの設計原理に関して具体的なコード例も交えつつ見てきました。どれも”原理”というだけあり、言葉にするととてもシンプルで単純明快ですが、実際にこの原理に沿ったコードを自然と書けるようになるためには、時間がかかるような気がしています。
レビュアー/レビューイの立場問わず、既存コードを読む際にもこの原理に沿っているかを常に意識し、私自身も読みやすく保守性のあるコードをかけるようになっていきたいと思います。

参考文献

プリンシプル オブ プログラミング3年目までに身につけたい一生役立つ101の原理原則


Schooでは一緒に働く仲間を募集しています!

20
6
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
20
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?