継承を使うべきか？レビューアーが設計判断を問う実務コード

レビューをしていると、ふとした設計に「それ、本当に必要か？」と首をかしげる瞬間がある。
その問いは往々にして、継承を使った設計に対して生まれる。

今回のレビュー対象は、決済処理を「テンプレートメソッドパターン」で実装したコード。
決して間違っているわけではない。しかし、レビューアーとしてそれを良い設計とみなすか、改善を促すかは一段深い判断が求められる。

なぜ今「継承を使うべきか」が問われるのか？#

Javaの教科書では、共通処理を親クラスに集約し、個別の処理はサブクラスに委ねるテンプレートメソッドパターンが紹介される。

これは理屈としては美しい。ただし現場では、それが“意図された設計”なのか、単に“書きやすいから継承した”だけなのかを見極めなければならない。

とくにレビューアーの立場では、以下のような視点が重要になる：

• 継承により呼び出し順や振る舞いが隠蔽されていないか
• 「親に処理を書けば共通化できるから」という構文主導の発想に陥っていないか
• 将来の拡張で構造的制約を生む設計になっていないか

つまり、コードが「正しく動いている」ことと「正しく設計されている」ことは別問題である。

現状設計の意図：コードの背後にある前提#

今回の実装者は、「ログ出力」「支払い処理本体」という流れを共通化したかったようだ。

それは素朴な目的である。共通部分を親クラスに、個別部分をサブクラスに…という流れは、OOPを学んだ者であれば誰しも一度は書いた経験があるはずだ。

ただし、目的と手段が一致していないケースでは、後々コードが“伸びない”構造になってしまう。
ではまず、今回の設計が何を意図しているかをシーケンス図で見てみよう。

現状コードの設計意図（シーケンス図）#

読み解きポイント#

• PaymentProcessor（抽象クラス）にログ処理（start/end）が存在する
• サブクラス（ここでは CreditCardProcessor）が doProcess() をオーバーライドして処理本体を実装
• processPayment() の中で呼び出し順が固定されている
• 利用者（PaymentService）は PaymentProcessor のサブクラスを生成して使う

つまり、テンプレートメソッド構造による呼び出し順制御＋共通化が目的と思われる。

一見すると「うまくまとまっている」ように見える。
だが、この設計には“意図して使った継承”と“都合で使った継承”の境界が曖昧な危うさが含まれている。

実装コード（抜粋）#

まず、親クラスとなる PaymentProcessor から見ていく。

public abstract class PaymentProcessor {
    public void processPayment(int amount) {
        logStart();
        doProcess(amount);
        logEnd();
    }

    protected abstract void doProcess(int amount);

    protected void logStart() {
        System.out.println("=== Payment started ===");
    }

    protected void logEnd() {
        System.out.println("=== Payment completed ===");
    }
}

次に、クレジットカードによる支払い処理。

public class CreditCardProcessor extends PaymentProcessor {
    @Override
    protected void doProcess(int amount) {
        System.out.println("Processing credit card: " + amount + " JPY");
    }
}

最後に、決済方法を選んで支払いを行うクラス。

public class PaymentService {
    public void pay(String method, int amount) {
        PaymentProcessor processor;
        if ("card".equals(method)) {
            processor = new CreditCardProcessor();
        } else {
            throw new IllegalArgumentException("Unsupported method");
        }

        processor.processPayment(amount);
    }
}

“継承”という構造に漂う違和感#

このコードは動く。実際、実装者の意図は反映されている。

では、レビューアーとしてこれを見たときに、「いいですね」と言ってスルーしてしまって良いのか？
継承というのは、使った瞬間に“構造的な硬直”を持ち込む設計要素だ。レビューで見過ごせば、次にコードを拡張する人が「この継承、壊していいんだっけ？」と躊躇することになる。

継承構造に漂う違和感はどこから来るのか？#

前回紹介したコードを見て「構造として綺麗だし、これで良いのでは？」と思った人も少なくないだろう。
テンプレートメソッドパターンを使い、ログ処理と個別処理が分離されている。if文で分岐しながら各Processorを使っている。動作も正しい。
では一体何が問題なのか？

「動くコード」≠「拡張可能な設計」#

このレビューは、構文やロジックの正しさを見る場ではない。
レビューアーとしての仕事は、このコードが次に“壊れるとしたらどこか”、その壊れ方に“設計意図が耐えられるか”を予測することにある。

もし、以下のような変更要求が将来入ったとしたら？

• 「決済ログをDBに保存したい」
• 「QRコード決済を追加したい」
• 「決済の前に外部認証を挟みたい」

こうした要望が出たときに、今の継承構造はどうなるか？

・ログ処理がDB依存になると `PaymentProcessor` がインフラ層に近づいてしまう
・外部認証の導入により `processPayment()` の前後処理が拡張不能になる
・新決済方式追加のたびに if-else が膨らみ `PaymentService` の責務が崩れる

この時点で、今の継承構造は「表面上の整合性」によって未来の拡張を阻んでいる構造だと分かる。

なぜこの継承は設計として危ういのか？#

Javaにおける継承は、「同一の責務を持つ共通の振る舞いを提供する」手段であって、単にコードをまとめるための仕組みではない。

つまり、PaymentProcessor という親クラスが存在する以上、レビューアーとしてはこう問い直す必要がある：

• 親クラスは何の責務を担っているのか？
• その責務は、子クラス共通であることが明確か？
• 呼び出し側（PaymentService）は、なぜその順番で処理すべきと決まっているのか？

これに明確な答えがないとすれば、それは「継承を使う意味」が実装側に委ねられてしまっている、“設計放棄の継承”になっている可能性がある。

レビューアーが抽出すべき構造上の問題#

各問題点のレビュー解説#

■ 高：テンプレートメソッドによる“順序の強制”が拡張を妨げる#

public void processPayment(int amount) {
    logStart();
    doProcess(amount);
    logEnd();
}

この形式は一見、読みやすく、安全にも見える。
だが、将来的に「前処理を分岐させたい」「後処理で認証を入れたい」となった瞬間、
このテンプレート構造がすべての子クラスに同一の処理順を強制する“足かせ”となる。

@Reviewer: 呼び出し順が固定されており、外部条件によって前処理/後処理を変えたくなった場合に設計上柔軟性がありません。責務を委譲し、呼び出し順を管理する側と処理の中身を分離する方向を検討してください。

■ 中：状態を持たない抽象クラスは“構文的継承”の温床#

public abstract class PaymentProcessor {
    // ...
}

このクラスが 状態（フィールド）も、実質的な制御ロジックも持たない場合、
継承してまで共通化すべき内容があるかを再確認すべきだ。

たとえば次のようにインタフェース＋委譲で代替可能ではないか？

public interface PaymentHandler {
    void handle(int amount);
}

■ 低：決済方式の分岐が PaymentService に埋め込まれている#

if ("card".equals(method)) {
    processor = new CreditCardProcessor();
}

これは現時点では2分岐だから問題が見えにくいが、
方式が3、5、7…と増えたとき、保守性とテスト容易性が低下する。

FactoryパターンやDI、さらにはMapによるStrategy登録での差し替え設計など、責務分離した方式選択の実装が可能である。

この構造、本当にレビューで指摘すべきか？#

レビューアーにとって最も難しい判断の一つは、「これは指摘すべき問題なのか？ それとも文脈として受け入れるべきものか？」という境界線だ。

前回までで挙げた継承構造の懸念点は、いずれも「壊れてはいないが、今後の拡張に耐えない」タイプのものだった。
つまり、今すぐバグになるわけではないが、設計判断としては弱い。

こういったケースこそ、レビューアーの判断技術と伝え方の設計が試される。

指摘する／しないをどう判断するか？#

指摘「すべき」ケース#

• 他の箇所ですでに別設計が採用されており、設計の一貫性を損なっている
• この構造が今後の拡張に明確な障害となる要求が見えている
• サブクラス追加がチーム内で頻発しており、可読性・維持性にコストが顕在化している

一旦スルーすべきケース（ただし記録に残す）#

• 既存コードベースが広くこのスタイルを採用しており、全体で揃えている意図がある
• リファクタリングの時間が明らかに確保されておらず、短期的なリリース優先の状況
• 実装者が明示的に「簡易実装で、後で見直す」とコメントに記している

実際にどうコメントを書くか：レビューコメント設計#

では、レビューアーとして今回の構造に対して建設的かつ判断の軸を明示するコメントを書くにはどうすれば良いか？

以下に、初級〜中級〜上級向けのコメント例を段階別に提示する。

初級メンバー向け：やさしく背景を共有する#

@Reviewer: 継承構造によって `processPayment()` の呼び出し順が固定されているため、今後の拡張（たとえば前処理の追加や外部連携）に柔軟に対応できるか少し懸念しています。

将来的に処理の前後に条件分岐や非同期処理などが入りそうであれば、Strategyなどへの分離も検討してみてください。

中堅メンバー向け：設計意図の妥当性を問いかける#

@Reviewer: `PaymentProcessor` のテンプレート構造は一見整理されていてよく見えるのですが、前後処理の順序が親クラスに固定されているため、設計としての意図と制約が明確かどうか気になりました。

この順序で固定することに業務上の意味（たとえばログ出力の責務固定など）があるのであれば、その設計意図をコメントやドキュメントに明記しておけると、後続メンバーが安心して継承できます。

上級開発者・設計者向け：設計再考を提案#

@Reviewer: `PaymentProcessor` がテンプレートメソッドで制御構造を持っている点、意図と整合していれば問題ないのですが、状態も制約も持たない設計としては継承のコストの方が上回る懸念があります。

共通化の粒度が構造を硬直させるリスクがあるため、制御順序の外出し（Caller責務化）や、Strategy＋Loggingの明示的委譲による分離構成も視野に入れて再設計できると拡張性が上がると思います。

コメント設計の3つのポイント#

1. “実装そのもの”ではなく、“設計意図と構造”に言及する

   • 「このifは良くない」ではなく、「この分岐は構造に責任を埋め込んでいる」と表現する

2. 「正しい設計」を押しつけるのではなく、「設計判断の補助線」を引く

   • 例：「この構造を選んだ背景が何かあるなら補足してほしいです」

3. 受け取る側のレベルに応じてコメントを段階的に調整する

   • コードだけを見る人には処理順を、設計を考える人には責務構造を伝える

結び：レビューアーが担うべき設計判断支援#

今回のような「壊れていないけど設計にリスクがあるコード」に対しては、単なる“指摘”ではなく、“設計意図を言語化する支援”というスタンスが重要になる。

レビューアーは、実装者が無意識に選んでしまった構造に対して、「それってどういう意図で選んだ？」と問いを立て、設計の持続性に道筋をつける存在であるべきだ。

継承をやめるなら、どう設計するのが良いのか？#

ここまで読んでいただいた方は、「この継承は設計として適切か？」という問いに対して、
「うまく動いてはいるが、構造として伸びない」と感じたはずだ。

では、もしこの継承をやめるとしたら、代わりにどのような構造にすれば良いのか？
ここではレビューアーとして提案可能な設計パターンを紹介しつつ、実装コードの再構成案を提示する。

設計の方向性：継承ではなく“委譲＋責務の分離”へ#

今回のケースにおける設計の問題点は、以下の3点に集約される：

1. ログ処理と業務処理が呼び出し順で固定されている
2. 処理の「中身」だけでなく「流れ」も継承で強制されている
3. 決済方式の追加が構造レベルで柔軟に行えない

これに対して、以下のような構造を採用することで、拡張性と保守性が両立しやすくなる。

改善案：Strategyパターン＋明示的な制御フロー設計#

設計方針#

• ログ処理は外から注入（責務の明確化）
• 決済方式の処理はインタフェース化し、外部から差し替え可能に
• 処理順序は呼び出し側が明示的に制御

Step 1: 処理のインタフェース化#

public interface PaymentHandler {
    void handle(int amount);
}

Step 2: 決済方式の具象実装#

public class CreditCardHandler implements PaymentHandler {
    @Override
    public void handle(int amount) {
        System.out.println("Processing credit card: " + amount + " JPY");
    }
}

Step 3: ログ処理を担うサービス#

public class PaymentLogger {
    public void start() {
        System.out.println("=== Payment started ===");
    }

    public void end() {
        System.out.println("=== Payment completed ===");
    }
}

Step 4: 呼び出し側が順序と責務を制御#

public class PaymentService {
    private final Map<String, PaymentHandler> handlers;
    private final PaymentLogger logger;

    public PaymentService(Map<String, PaymentHandler> handlers, PaymentLogger logger) {
        this.handlers = handlers;
        this.logger = logger;
    }

    public void pay(String method, int amount) {
        PaymentHandler handler = handlers.get(method);
        if (handler == null) {
            throw new IllegalArgumentException("Unsupported payment method: " + method);
        }

        logger.start();
        handler.handle(amount);
        logger.end();
    }
}

利点まとめ#

レビューアーの提案は「設計判断の再提示」である#

レビューアーは、「このコードのここが良くない」という粗探しをする役割ではない。
むしろ、「この設計選択が今後どう響くか」を示し、「他の選択肢をどう比較するか」を言語化できる人こそが、信頼されるレビューアーだ。

今回のような設計レビューでは、継承をやめることが目的ではない。
継承の意味を問い直し、設計の意図と整合していない部分があれば、それを補う再設計の方向性を提示すること。
それがレビューアーの本質的な価値になる。

総まとめ：レビューアーとして“継承”をどう扱うか#

• 継承を使っているコードを見たら、「なぜ継承にしたのか？」を必ず自問する
• 親クラスが状態を持たない／制御構造だけ持っている場合、委譲への移行を検討する
• 処理順序が固定されているが理由が明記されていないときは、拡張性へのリスクがある
• レビューでは、「構造が壊れていないか」だけでなく、「構造が息をしているか」を見る