4
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@ryamate(やまて)in株式会社スマレジ

実際に受けたコードレビューから学ぶ！ドメイン駆動設計、クリーンアーキテクチャ

4
Posted at

はじめに

ここ数ヶ月の業務で、バックエンド（PHP/Laravel）の実装をする機会が多くなり、ドメイン駆動設計（DDD）とクリーンアーキテクチャについて、しっかりコードレビューを受けられる機会を得ました。

どのようなコードレビューを受けてきたかをまとめて、学びをアウトプットして共有し、自分自身としては知識を定着させる機会にできたらと思います。

私について

  • ガッツリ実装業務をするようになってからは 3 年ほど経過しているけど、バックエンドの新規開発の実装は初めて
  • DDD の入門書は読んだことはあったけど、DDD、クリーンアーキテクチャについて、業務で深く考える機会なし
  • 実際のレビューを通して、わかることも少しずつ増えてきたところ（指摘はまだ受ける）

想定する読者

以下のような方を想定しています。

  • DDD、クリーンアーキテクチャの実装を求められているけどまだわかっていない状況の方

目次

DDD、クリーンアーキテクチャに関して指摘するときのチェックポイントについて、レビュアーの方に共有してもらったので、その内容をまとめて、実際に受けたコードレビューをまとめます。

コードレビューの根拠としている書籍などの参考資料

何から逸脱していてはいけないのかの根拠についてですが、

DDD、クリーンアーキテクチャに関するコードレビューでの指摘の根拠は、基本的に以下の参考資料の内容を根拠にしている、とのことです。

聖書（参考書籍）

① クリーンアーキテクチャ: Clean Architecture 達人に学ぶソフトウェアの構造と設計

② ドメイン駆動設計: エリック・エヴァンスのドメイン駆動設計 | 翔泳社

参考書籍

③ ドメイン駆動設計入門 ボトムアップでわかる！ドメイン駆動設計の基本 | 翔泳社

よくまとまっている記事

④ クリーンアーキテクチャ完全に理解した

⑤ 実装クリーンアーキテクチャ - Qiita

ちなみに私は聖書の２冊はまだ読んでおらず、②を買って読み始めたところです。難しい。

また、⑤の記事冒頭の動画がとても分かりやすく入門の動画としておすすめです。私は時折通勤時などに見返して、脳に馴染ませています。

↓こちらの動画

どのような逸脱を指摘するかの基準

根拠となる設計から逸脱していたらどのようなものでも指摘するわけではなく、設計通り実装することのコストや実益などを考えて、スルーする部分もあります。

指摘の基準は以下です。

  • ドメイン駆動設計から外れた実装
    • ValueObject や Entity を使うべきところで使っていない など
  • クリーンアーキテクチャの依存性の方向が逆になっている
    • UseCase 層で RepositoryInterface ではなく Repository の実装を直接呼んでいる など
    • 実益の少ない教義的な部分はスルーすることもある
      • UseCase の Interface を作っていない など
  • 可読性に悪影響がある、整合性境界が崩れる
    • 整合性境界が崩れる例は、本来ひとつの集約内で完結すべき更新処理が、複数の Repository 呼び出しや別の集約操作に分割されてしまっている など

上記に逸脱する理由がなく逸脱がある場合に指摘をする、という基準です。

新規プロダクトでは基本的に逸脱する必要性はないものが多く、逸脱するなら理由が必要です。

（逸脱している理由をレビューコメントで聞かれるのですが、聞かれたところで私の理解不足での逸脱ばかりでした、、）

また、スケジュールに余裕がない状況ではあるので、基準は常に一定でもなくバランスを取っていくものかと思っています。

気になる逸脱もあるけど、リリースまでは致命的でないものは後回しにして、落ち着いたら修正しよう、というような判断もあると思います。

ただし、スルーすると保守性が失われうようなものは必ず指摘、という方針でした。

実際に受けたレビュー内容

ここからは、実際に受けたレビュー内容を、DDDとクリーンアーキテクチャそれぞれ 3 つずつ紹介していきます。

ドメイン駆動設計（DDD）の観点からの指摘

クリーンアーキテクチャの観点からの指摘

※ コード、レビュアーのコメントについては記事にするにあたって編集しています

ドメイン駆動設計（DDD）の観点からの指摘

1. ユビキタス言語の徹底

複数回の指摘を受けたのが、ドメイン用語、ユビキタス言語についてです。

// ❌ 最初の提案
class GetStaffResourcesUseCase { ... }

// ❌ 次の提案
class GetStaffCreationContextUseCase { ... }

// ✅ 最終決定
class GetStaffCreationChoicesUseCase { ... }

レビュアーのコメント：

「ここで使っている Resources というのは何を指してますか？そういうドメインの用語は無いと思ってます。いわゆるユビキタス言語ではないと思います。」

「また、Resource という単語は REST や Laravel API など多数の文脈で利用されるので混乱を招きます。」

編集モーダルの選択肢のある項目の選択肢を取得する API のユースケースについての命名についてのコメントでした。

最適なユビキタス言語にすぐに当てはめられるような用語がなく、今後も考える余地ありで、修正はベターなものにしておこうという判断になりました。

学んだこと：

  • コード上の単語は、ユーザー・ビジネスサイドと共通の単語を使うことを意識する
  • 開発側が勝手に作ったユビキタス言語ではない用語を使うは NG
  • 技術的分類より機能的分類を優先する
  • 適切なドメイン用語がない場合は、画面の文脈などから命名を考えることはある

正直言って、最初は単なる命名の細部の指摘かと思っていました。

しかし、後々に影響する悩む価値のあるポイントであることを理解しました。

補足）類似の指摘：

同様に、値オブジェクトのクラスに書いた 有効・無効、Valid・Invalid という技術用語に基づいた命名も、ビジネス用語ではないためドメイン層では使用を避けるべきと指摘を受けました。

ドメイン層では常に「仕様」を表現することを意識する必要があることを学びました。

2. Repository 引数はドメインオブジェクトのみ

続いては、Repository 設計の基本原則についてです。

// ❌ 指摘された実装
interface StaffRepositoryInterface
{
    public function save(
        Staff $staff,
        array $operationIds,  // ← 素のarrayを渡していた
        array $tagIds
    ): void;
}

// ✅ 修正後
interface StaffRepositoryInterface
{
    public function save(Staff $staff): void;
}

レビュアーのコメント：

「Repository の引数は Entity のみにすべきです。ドメイン駆動設計の基本的なルールで、Entity での表現ができてないです。ドメイン層は DB のテーブルがいくつに分かれているかを一切知るべきではないと思います。」

学んだこと：

  • Repository 引数は基本的に、Entity や ValueObject などのドメインオブジェクトになる
    • Repository は、集約単位で永続化する責務を持つ
      • 配列やプリミティブで直接受け取る設計は、集約単位で永続化を崩してしまう危険がある
    • 整合性は集約単位で保証されるべきで、その単位を崩さないことが重要
  • ドメイン層（ここでは RepositoryInterface）は DB テーブル構造を知るべきではない（Repository の実装に委ねる）

今回の指摘箇所は、整合性を保つための重要な DDD の基本原則で、逸脱すべきではないポイントだと学びました。

3. Factory で更新処理をしてはいけない

次に、Factory の責務についてです。

// ❌ 指摘された実装
class StaffFactory
{
    public function update(Staff $staff, StaffName $name, Email $email): Staff
    {
        $staff->changeName($name);
        $staff->changeEmail($email);

        return $staff;
    }
}

レビュアーのコメント：

「Factory は生成が責務なので、更新処理を持つのは違和感があります。」

「更新は Entity 自身の振る舞いとして表現されるべきです。」

Factory は、複雑な生成処理をカプセル化するための概念です。つまり責務は生成のみで、更新は Entity 自身の振る舞いとして表現されるべきです。

// ✅ 望ましい形（Entity に振る舞いを持たせる）
class Staff
{
    public function changeName(StaffName $name): void
    {
        $this->name = $name;
    }

    public function changeEmail(Email $email): void
    {
        $this->email = $email;
    }
}

そして UseCase などからは、Factory を経由せず直接 Entity の振る舞いを呼び出します。

$staff->changeName($name);
$staff->changeEmail($email);

$staffRepository->save($staff);

学んだこと：

  • Factory の責務は生成に限定する
  • 更新は Entity の振る舞いとして表現する

Factory の責務は？といった、役割の違いを理解しておかないと、すぐにおかしな設計で実装してしまうことに繋がりますね…

クリーンアーキテクチャの観点からの指摘

4. Controller 層での Entity 生成は禁止

次に、クリーンアーキテクチャの層分離についての指摘です。

// ❌ 指摘された実装（Controller層）
class CreateStaffAction
{
    public function __invoke(Request $request)
    {
        // Controller層でエンティティを直接生成していた
        $entity = new Staff(...);
    }
}

※ サンプルコードは Single Action Controller パターンを採用しており、クラス名は XxxAction となっていますが、役割としては一般的な Controller 層に相当します。

レビュアーのコメント：

「Entity はドメイン層なので Controller で生成されるものではないと思います。」

「Entity は Factory で生成しないと外部依存を切り離せないです。」

学んだこと：

  • Entity 生成は Controller 層ではなく、Factory で行う（UseCase から Factory を呼び出す）
  • クリーンアーキテクチャの層分離を厳格に守る

補足として、ValueObject 生成については、Controller 層でも許容される（内向き依存が保てる）と説明を受けました。ValueObject は、副作用はないため問題はないと理解しました。

5. Controller 層に private メソッドを定義すべきではない

責務混在の兆候について指摘されました。

// ❌ 指摘された実装
class GetStaffsAction
{
    public function __invoke(Request $request)
    {
        $data = $this->transformData($rawData);
        return response()->json($data);
    }

    private function transformData(array $data): array
    {
        // 変換処理...
    }
}

レビュアーのコメント：

「このメソッドは Controller が持つべきではないです。private メソッドを定義する時点で責務が混在していると思ってください。」

学んだこと：

  • Controller クラスはシンプルなパイプライン役に徹する
  • private メソッドの存在は責務混在の兆候
  • 複雑なロジックは適切な層（UseCase、Domain Service 等）に分離する

6. トランザクションはどの層で張るべきか？

最後の一つですが、これは最も指摘からの学びは多かったなと思うトピックでした。

段階1： Repository 層での誤り

// ❌ Repository層でトランザクション
class StaffRepository
{
    public function save(Staff $staff): void
    {
        DB::beginTransaction(); // ← ここで張っていた
        // 保存処理...
        DB::commit();
    }
}

レビュアーのコメント：

「トランザクションを Repository のなかで貼るのはだめですね。このあとの処理でコケたときに不正なレコードができることになります。」

段階2： UseCase 層での誤り

// ❌ UseCase層でトランザクション
class CreateStaffUseCase
{
    public function execute(StaffId $id, ...): void
    {
        DB::beginTransaction(); // ← DBファサードへの依存
        // 処理...
        DB::commit();
    }
}

レビュアーのコメント：

「クリーンアーキテクチャにおける UseCase もドメイン駆動設計におけるアプリケーションサービスも DB への依存をするべきではないのでトランザクションはこのクラスに書くべきではないですね。」

「Controller 層でトランザクション貼ることになると思います。」

段階3： Controller 層での正解

// ✅ Controller層でトランザクション
class CreateStaffAction
{
    public function __invoke(CreateStaffRequest $request)
    {
        try {
            DB::beginTransaction();

            ($this->useCase)($request->staffId(), ...);

            DB::commit();
            return response()->json(['message' => 'Created'], 201);
        } catch (\\\\Exception $e) {
            DB::rollBack();
            throw $e;
        }
    }
}

学んだこと：

  • UseCase 層は DB の技術的詳細（トランザクション）に依存すべきではない
  • DDD 的には UseCase で貼るのが自然だが、実装の現実とのトレードオフを考慮する

補足）理論と実践のトレードオフ：

この指摘を受けて、「ドメイン駆動設計入門」で学んだ通り、UseCase で張るべきではないか、と疑問を持ちました。

レビュアーからは、以下のような回答をもらいました：

DDD 的にはトランザクションは UseCase で貼るのが自然な責務だと思っているけど、このプロジェクトでは以下のトレードオフのために Controller で貼るようにしている。

  • Domain フォルダと Adapter フォルダを明確に分けて、純粋な PHP にのみ依存する層とフレームワークやライブラリ、インフラに依存する層をフォルダで分けている
  • deptrac などのツールでも依存のチェックをしている
  • UseCase に DB::beginTransaction を書いてしまうとDBファサードへの依存を許してしまい、クエリ操作の検出が難しくなる
  • メリットの大きさは、「メンバー全員にわかりやすいフォルダ分けをして依存関係を守ってもらうこと」 > 「ドメイン駆動設計に厳密に準ずること」

理論的な正しさと、実装レベルでの実践のしやすさのトレードオフを考慮することの重要性を学びました。

「ドメイン駆動設計入門」によれば、C# であれば @Transactional のような AOP で実現できる、とありましたが、PHP では別の方法で実現する必要があり、実装コストを考えて今回の判断に至っているため、言語によるところもあります。

以上、実際に受けたコードレビューなので内容に項目の偏りはありますが、ドメイン駆動設計、クリーンアーキテクチャを理解する上でのヒントになったものをピックアップしました。

おわりに

実際に私が受けた、ドメイン駆動設計とクリーンアーキテクチャについてのコードレビューについてまとめました。

入門書は読んだものの、実装レベルでどう落とし込むかわからなかった私にとって、実際のコードレビューを通じて学べることはとてもありがたい機会です。

難しいとは感じますが、少しずつでも理解を深めていきたいと思います。

この記事が、同じようにドメイン駆動設計、クリーンアーキテクチャの実践で悩んでいる方の参考になれば幸いです。

ありがとうございました。

4
1
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
Sign upLogin
4
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?