「良いコード悪いコードで学ぶ設計入門」を読んで入社時研修で作ったゲームをリファクタリングしてみた

Last updated at 2025-08-20Posted at 2025-08-15

注意: この記事は「良いコード悪いコードで学ぶ設計入門」の内容をもとに記載しており、この記事の内容が必ずしも正解というスタンスでは書いていません。反対意見については「最後に」の章をご覧ください。

はじめに

こんにちは、新卒２年目エンジニアの戸田です。
入社から１年が経ちオブジェクト指向言語の記法についてはある程度理解が進んできましたが、プログラム全体をどう作り込むべきかまだ分かっていません。そこで設計の初歩を勉強するために「良いコード/悪いコードで学ぶ設計入門」という書籍を読んでみました。この記事ではその内容を共有するとともに、アウトプットとして入社時研修の課題で作成したゲームをリファクタリングする流れを紹介します。

「良いコード/悪いコードで学ぶ設計入門」は、アンチパターンが引き起こす弊害とそれらの予防策を通して、バグを引き起こしにくい構造を学んでいける書籍です。新米エンジニア的には耳が痛い内容も多く記載されていました。この記事を読んで「このアンチパターン、やった覚えがある」と思った方にはきっと刺さるはずなので、ぜひ手に取ってみてください。

Amazonリンク：
改訂新版-良いコード／悪いコードで学ぶ設計入門-―保守しやすい-成長し続けるコードの書き方

書籍の内容紹介：様々な弊害を引き起こす悪魔的なコード
悪魔へ対処するための基本原則
リファクタリング対象: すごろくゲーム
リファクタリング前のコードに潜む悪魔
リファクタリングの方針
リファクタリング後のコード
リファクタリングの効果確認：仕様追加
まとめ

1. 書籍の内容紹介：様々な弊害を引き起こす悪魔的なコード

書籍の特徴

「良いコード/悪いコードで学ぶ設計入門」の最大の魅力は、アンチパターンを「悪魔」として擬人化した親しみやすい解説にあります。

堅めの技術書では抽象的な理論から入ることが多いように思いますが、この書籍では具体的なコードをベースに「悪魔」たちが紹介されています。それぞれがどんな弊害を起こすのか、どう退治すれば良いのかが攻略本のように説明されており、難解な内容に興味を失うこともなく楽しく読み進めることができます。対象読者は『オブジェクト指向プログラミング言語の基礎知識はあるものの、設計がわからない/自信がない開発者』となっており、初級者でもとっつきやすい構成になっています。

主要な悪魔とその対処方法

ここでは、書籍で紹介されている悪魔とその対策をいくつかピックアップして紹介します。

生焼けオブジェクト

インスタンス変数を利用者側で初期化しないと使い物にならないクラスです。中身を知らない開発者が誤って利用することで、初期化されていないインスタンス変数を持ったオブジェクトが誕生し、バグの発生原因になります。

// 悪い例：生焼けオブジェクト
public class Money {
    int amount;      // 未初期化状態
    String currency; // 未初期化状態
    
    public Money() {
    }
}

対処方法：完全コンストラクタ

コンストラクタで全てのインスタンス変数を初期化するよう作り込みます。

// 良い例：完全コンストラクタ
public class Money {
    private final int amount;
    private final String currency;
    
    public Money(int amount, String currency) {
        this.amount = amount;
        this.currency = currency;
    }
}

低凝集

関連するデータとロジックがバラバラになっている状態です。インスタンス変数（データ）に関するバリデーションや計算処理（ロジック）がクラス自体に備わっていないと、呼び出す側で実装する必要が出てくるため、同じような処理が複数の場所で実装されることになります。この状態を低凝集と呼び、修正時の影響範囲が把握できずバグの原因となります。
書籍では低凝集なクラスの例として、データのみを持ちロジックを持たない「データクラス」が紹介されています。

// 悪い例：低凝集
public class OrderProcessor {
    public void processOrder(int amount, String currency) {
        // バリデーションが他のクラスに散在
        if (amount < 0) {
            throw new IllegalArgumentException("金額は0以上");
        }
        if (!currency.equals("JPY")) {
            throw new IllegalArgumentException("JPYのみ対応");
        }
        
        int tax = amount * 10 / 100;  // 消費税計算も散在
        int total = amount + tax;
    }
}

対処方法：データとロジックをまとめて凝集度を向上

関連するデータとロジックを一つのクラスにまとめて、高凝集な設計にします。また、インスタンス変数にバリデーションを設定し、不正なインスタンスが発生し得ない状況を作ります。
以下ではデータクラスにバリデーションやロジックを追加して「値オブジェクト(Value Ovject)」として運用することで高凝集にしています。

// 良い例：値オブジェクト
public class Money {
    private final int amount;
    private final String currency;
    
    public Money(int amount, String currency) {
        // バリデーションをMoneyクラス内で実施
        if (amount < 0) {
            throw new IllegalArgumentException("金額は0以上");
        }
        if (!currency.equals("JPY")) {
            throw new IllegalArgumentException("JPYのみ対応");
        }
        this.amount = amount;
        this.currency = currency;
    }
    
    public Money addTax(TaxRate taxRate) {
        int taxAmount = this.amount * taxRate.getRate() / 100;
        return new Money(this.amount + taxAmount, this.currency);
    }
}

可変による意図せぬ影響

インスタンス変数が可変（ミュータブル）な状態です。オブジェクトの状態が予期せず変更されることで、他のコードが期待していた値と異なる値になり、デバッグが困難なバグが発生します。また、マルチスレッド環境では競合状態が発生し、データの整合性が保てなくなる可能性があります。

// 悪い例：可変オブジェクト
public class Money {
    private int amount;
    private String currency;
    
    public Money(int amount, String currency) {
        this.amount = amount;
        this.currency = currency;
    }
    
    public void add(int additionalAmount) {
        this.amount += additionalAmount;  // 既存オブジェクトの状態を変更
    }
    
    public void setAmount(int amount) {
        this.amount = amount;  // 外部から状態変更が可能
    }
}

対処方法：不変オブジェクト（イミュータブル）の実装

インスタンス変数を不変（final）にし、状態を変更する際は新しいインスタンスを生成して返します。

// 良い例：不変オブジェクト
public class Money {
    private final int amount;
    private final String currency;
    
    public Money(int amount, String currency) {
        this.amount = amount;
        this.currency = currency;
    }
    
    public Money add(int additionalAmount) {
        // 新しいインスタンスを生成して返す
        return new Money(this.amount + additionalAmount, this.currency);
    }
    
    public int getAmount() {
        return amount;  // 値の取得のみ可能
    }
}

多重ネスト

if文やfor文が何重にも重なっている状態です。ネストが深くなるほどコードの可読性が大幅に低下し、どの条件分岐がどの処理に対応しているか把握が困難になります。また、バグの発生箇所の特定や修正も困難になり、保守性が著しく悪化します。

// 悪い例：多重ネスト
public void processOrder(Order order) {
    if (order != null) {
        if (order.getCustomer() != null) {
            if (order.getCustomer().isPremium()) {
                if (order.getTotalAmount() > 10000) {
                    if (order.getItems().size() > 0) {
                        // 深いネストで処理が埋もれている
                        applyPremiumDiscount(order);
                    }
                }
            }
        }
    }
}

対処方法：早期return（ガード節）の活用

条件を満たさない場合に早期returnすることで、ネストを解消し可読性を向上させます。

// 良い例：早期return
public void processOrder(Order order) {
    if (order == null) return;
    if (order.getCustomer() == null) return;
    if (!order.getCustomer().isPremium()) return;
    if (order.getTotalAmount() <= 10000) return;
    if (order.getItems().size() == 0) return;
    
    // 処理が明確に見える
    applyPremiumDiscount(order);
}

switch文の重複

同じような条件分岐のswitch文が複数の箇所に重複して存在する状態です。新しい条件が追加されるたびに全ての関連するswitch文を修正する必要があり、修正漏れによるバグが発生しやすくなります。また、条件分岐のロジックが散在することで、仕様変更時の影響範囲の把握が困難になります。

// 悪い例：switch文の重複
public class OrderProcessor {
    public double calculateDiscount(String membershipType, double amount) {
        switch (membershipType) {
            case "BRONZE": return amount * 0.05;
            case "SILVER": return amount * 0.10;
            case "GOLD": return amount * 0.15;
            case "PLATINUM": return amount * 0.20;
            default: return 0;
        }
    }
    
    public String getMembershipBenefit(String membershipType) {
        switch (membershipType) {
            case "BRONZE": return "ポイント2倍";
            case "SILVER": return "ポイント3倍 + 送料無料";
            case "GOLD": return "ポイント5倍 + 送料無料 + 優先サポート";
            case "PLATINUM": return "ポイント10倍 + 送料無料 + 専用サポート";
            default: return "特典なし";
        }
    }
}

対処方法：ストラテジパターンとMapの活用

各条件の処理を共通のインタフェースを実装した独立したクラスに分離し（ストラテジパターン）、Mapを使って条件とクラスを対応付けます。パラメータ追加時はインタフェースを修正するため、各クラスはインタフェースに沿った実装となり修正漏れを防げます。

// 良い例：ストラテジパターン + Map
public interface MembershipStrategy {
    double calculateDiscount(double amount);
    String getBenefit();
}

public class BronzeMembership implements MembershipStrategy {
    public double calculateDiscount(double amount) { return amount * 0.05; }
    public String getBenefit() { return "ポイント2倍"; }
}

public class SilverMembership implements MembershipStrategy {
    public double calculateDiscount(double amount) { return amount * 0.10; }
    public String getBenefit() { return "ポイント3倍 + 送料無料"; }
}

public class OrderProcessor {
    private final Map<String, MembershipStrategy> strategies;
    
    public OrderProcessor() {
        strategies = Map.of(
            "BRONZE", new BronzeMembership(),
            "SILVER", new SilverMembership(),
        );
    }
    
    public double calculateDiscount(String membershipType, double amount) {
        return strategies.getOrDefault(membershipType, new NoMembership())
                        .calculateDiscount(amount);
    }
}

共通処理クラス（Common、Util）

CommonやUtilといった汎用的な名前のクラスに、関連性の薄い処理をまとめて配置する状態です。クラスの責務が不明確になり、どこに何の処理があるか把握困難になります。また、無関係な処理が一箇所に集まることで、一つの修正が予期しない箇所に影響を与えるリスクが高まります。

// 悪い例：共通処理クラス
public class CommonUtil {
    public static String formatDate(Date date) { /* 日付フォーマット */ }
    public static boolean validateEmail(String email) { /* メール検証 */ }
    public static int calculateTax(int amount) { /* 税計算 */ }
    public static void sendNotification(String message) { /* 通知送信 */ }
    public static String generateId() { /* ID生成 */ }
}

対処方法：責務に応じたクラス分割

機能ごとに責務を明確にしたクラスに分割し、具体的で意味のある名前を付けます。共通処理クラスを作る場合は、ログ出力や例外処理など全てのクラスで必ず必要になる処理のみを入れるようにします。

// 良い例：責務に応じた分割
public class DateFormatter {
    public static String format(Date date) { /* 日付フォーマット */ }
}

public class EmailValidator {
    public static boolean isValid(String email) { /* メール検証 */ }
}

public class TaxCalculator {
    public static int calculate(int amount) { /* 税計算 */ }
}

// 真に共通的な処理のみの例
public class Logger {
    public static void info(String message) { /* 全クラスで使用するログ出力 */ }
    public static void error(String message) { /* 全クラスで使用するエラーログ */ }
}

密結合（神クラス）

あらゆる責務のロジックが一つのクラスに集中している状態です。一つの変更が他の機能に影響を与えるリスクがあり、テストが困難で、複数人での並行開発も困難になります。
後ほど原則として紹介する「疎結合高凝集」の"高凝集"を意識しすぎて陥る罠でもあります。関連していそうなロジックを一つのクラスに集めすぎることで神クラスが出来上がります。

// 悪い例：神クラス
public class OrderManager {
    public void processOrder(String customerId, List<String> items) { /* 注文処理 */ }
    public void updateInventory(String itemId, int quantity) { /* 在庫管理 */ }
    public void processPayment(String paymentMethod, Money amount) { /* 決済処理 */ }
    public void sendEmail(String email, String orderId) { /* メール送信 */ }
}

対処方法：単一責任の原則による責務の分散

一つのクラスは一つの責任のみを持つように、責務を複数のクラスに分散します。インスタンス変数の中にクラス分割可能なものがないか注意して設計する必要があります。

// 良い例：責務を分散
public class OrderService {
    public void processOrder(Order order) { /* 注文処理のみ */ }
}

public class InventoryService {
    public void updateStock(String itemId, int quantity) { /* 在庫管理のみ */ }
}

public class PaymentService {
    public PaymentResult processPayment(PaymentRequest request) { /* 決済処理のみ */ }
}

密結合（継承）

継承を使用した際に、サブクラスがスーパークラスの内部実装に強く依存している状態です。スーパークラスの変更がサブクラスに予期しない影響を与え、継承階層が深くなるほど影響範囲が拡大します。また、スーパークラスの不適切な設計がサブクラス全体に波及し、柔軟性が失われます。

// 悪い例：継承による密結合
public class Weapon {
    protected int baseDamage;
    protected String type;
    
    public int calculateDamage() {
        return baseDamage + getTypeBonus();  // 内部実装に依存
    }
    
    protected int getTypeBonus() {
        return type.equals("SWORD") ? 10 : 0;
    }
}

public class MagicSword extends Weapon {
    private int magicPower;
    
    @Override
    protected int getTypeBonus() {
        // スーパークラスの実装に強く依存
        return super.getTypeBonus() + magicPower * 2;
    }
}

対処方法：コンポジション構造による疎結合化

継承ではなくコンポジション（構成）を使用し、必要な機能を独立したクラスとして組み込みます。コンポジション構造とは、「is-a関係（〜は〜である）」ではなく「has-a関係（〜は〜を持つ）」でクラス間の関係を構築する設計手法です。継承のように親クラスの実装に依存せず、必要な機能を持つオブジェクトを組み合わせて目的を達成します。

// 良い例：コンポジション構造
public class AttackCalculator {
    public int calculate(int baseDamage, int bonus) {
        return baseDamage + bonus;
    }
}

public class WeaponEffect {
    private final int effectValue;
    
    public WeaponEffect(int effectValue) {
        this.effectValue = effectValue;
    }
    
    public int getBonus() {
        return effectValue;
    }
}

public class MagicSword {
    private final int baseDamage;
    private final AttackCalculator calculator;  // コンポジション（変数として持つ）
    private final WeaponEffect magicEffect;     // コンポジション（変数として持つ）
    
    public MagicSword(int baseDamage, int magicPower) {
        this.baseDamage = baseDamage;
        this.calculator = new AttackCalculator();
        this.magicEffect = new WeaponEffect(magicPower * 2);
    }
    
    public int calculateDamage() {
        return calculator.calculate(baseDamage, magicEffect.getBonus());
    }
}

2. 悪魔へ対処するための基本原則

書籍では悪魔への具体的な対処法のほか、設計における基本原則が紹介されています。ここまで紹介してきた悪魔への対処法の根底には「疎結合高凝集」「単一責任の原則」といった共通の考え方が流れており、これらを意識することでより安全なでバグを生みにくい構造の設計が可能になります。

基本原則

疎結合高凝集：
関連性の高いデータとロジックを一箇所にまとめ（高凝集）、他のクラスとの依存関係は最小限に抑える（疎結合）という設計手法です。変更の影響範囲を局所化でき、バグの混入リスクを大幅に減らすことができます。この書籍の中で最も重要なキーワードです。
単一責任の原則：
「一つのクラスは一つの責任のみを持つべき」という考え方です。疎結合高凝集を実現するための考え方「SOLID原則」の一部として知られています。
尋ねるな命じろ：
疎結合高凝集を実現するための有名な考え方です。他のオブジェクトの状態を問い合わせて自分で判断するのではなく、そのオブジェクトに処理を委譲することで責任の所在が明確になり、ロジックの散在を防げます。

その他の重要原則

YAGNI原則：
YAGNI原則（You aren't going to need it）とは、「将来必要になるかもしれない機能を先回りして実装しない」という開発方針です。不要なコードの蓄積を防ぎ、システムの複雑性を抑制できます。

3. リファクタリング対象: すごろくゲーム

ここからは書籍の内容を参考に、入社時研修の課題で作成したすごろくゲームをリファクタリングしていきます。
この章では、私が実際に作成したゲームの内容とリファクタリング前の構造について説明します。

ゲームの内容

javaで書かれた、コンソールで動くシンプルなゲームです。仕様は以下の通りです。

ゲーム進行フロー

プレイヤーの数と名前を最初に入力する
各プレイヤーは毎ターン、エンターキーを押してサイコロを振り、振った数だけマスを進める
到着したマスがイベントマスだった場合、プレイヤーに対してイベントが発生する
プレイヤーが一人でもゴールすれば終了

イベントの種類

ラッキーマス: もう一度サイコロを振り、出た目の数先に進む
ざんねんマス: もう一度サイコロを振り、出た目の数後に戻る
超ざんねんマス: もう一度サイコロを振り、出た目の数ターンがスキップされる

主要なクラス構成

リファクタリング前のコードは以下のような構成になっています。

// ゲーム状態を管理するクラス
public class Game {
    int current_turn;
    int num_players;
    int num_squares;
    ArrayList<Square> field;
    ArrayList<Player> players;
    Boolean gameplay;
}

// プレイヤー情報を管理するクラス
public class Player {
    String name;
    int cite_index;  // 現在位置
    int wait_turn;   // 待機ターン数
}

// 各マスの情報を管理するクラス
public class Square {
    int event_code;  // イベントコード
}

// ゲーム進行とイベント処理を担当するクラス
public class eventManager {
    // ゲーム開始、ターン進行、イベント処理、入力処理、表示処理など
    // 226行に及ぶ巨大クラス
}

// エントリーポイント
public class Main {
    public static void main(String[] args) {
        Game game1 = new Game();
        EventManager event = new EventManager();
        
        int num_squares = 30;
        int event_proportion = 50;
        
        event.startGame(game1, num_squares, event_proportion);
        
        while(game1.gameplay) {
            event.playTurn(game1);
        }
        
        event.judgeWinner(game1);
    }
}

各クラスの役割

Game: ゲーム全体の状態情報を保持
Player: プレイヤーの情報（名前、位置、待機ターン数）を管理
Square: 各マスのイベント情報を管理
eventManager: ゲームの進行制御とあらゆる処理を担当
Main: エントリーポイント

4. リファクタリング前のコードに潜む悪魔

リファクタリング前のコードを紹介したところで、いよいよ悪魔退治に入ります。まずは書籍で学習した内容をもとに、実際に私のコードにどんな悪魔が住み着いているのか見ていきましょう。クラス別に深掘りして悪魔を炙り出していきます。

Game.java：生焼けオブジェクト

public class Game {
    int current_turn;
    int num_players;
    int num_squares;
    ArrayList<Square> field;
    ArrayList<Player> players;
    Boolean gameplay;
    
    public Game() {
        current_turn = 0;
        num_players=0;
        num_squares=0;
        gameplay = true;
    }
}

潜んでいる悪魔と弊害

生焼けオブジェクト: コンストラクタで初期化されない変数があり、Gameクラスが初期化されただけでは使い物にならない状態
弊害:
- 後からeventManagerによって設定される必要があり、設定忘れや不正な状態でのメソッド呼び出しのリスクがある

eventManager.java：神クラス

public class eventManager {
    // 226行に及ぶ巨大クラス
    public void startGame(Game g, int num_squares, int event_propotion) { /* ゲーム開始処理 */ }
    public void playTurn(Game g) { /* ターン進行処理 */ }
    public void judgeevent(Game g, Player p) { /* イベント処理 */ }
    public int diceRoll() { /* サイコロ処理 */ }
    public Square generateSauare(int code_range) { /* マス生成処理 */ }
    public Player generatePlayer() { /* プレイヤー生成処理 */ }
    public int inputPlayerNumber() { /* 入力処理 */ }
    public void figureOut(Game g) { /* 表示処理 */ }
}

潜んでいる悪魔と弊害

神クラス: ゲーム開始、ターン進行、イベント処理、入力処理、表示処理などのあらゆる責務のロジックが一つのクラスに集中
弊害:
- 一つの変更が他の機能に影響を与えるリスク
- テストが困難（全機能をまとめてテストする必要）
- 複数人での並行開発が困難
- コードの理解に時間がかかる

eventManager.java：尋ねるな命じろの違反による密結合

public void judgeevent(Game g, Player p) {
    Square current_cite = g.field.get(p.cite_index);
    if (current_cite.event_code == 1) {
        // イベント処理をeventManagerが行う
    }
}

潜んでいる悪魔と弊害

密結合: eventManagerがGameクラスの内部構造に深く依存
尋ねるな命じろの違反: eventManagerがSquareの状態を問い合わせて判断
弊害:
- Gameクラスの内部構造変更時にeventManagerも修正が必要
- オブジェクト指向の利点を活かせない
- 処理の責任が曖昧

Player.java, Square.java：低凝集とデータクラス

public class Player {
    String name;
    int cite_index;  // 位置
    int wait_turn;   // 待機ターン
}

public class Square {
    int event_code;  // イベントコード
}

潜んでいる悪魔と弊害

低凝集: データのみを持ち、データに関するロジックはeventManagerに散在
プリミティブ型の多用: int cite_index、int event_codeなど、型による安全性の欠如
弊害:
- プレイヤーの移動ロジックがeventManagerに分散
- 不正な値の設定を防げない
- コードの意図が不明確

全体：マジックナンバーと命名の問題

本記事では具体的に悪魔として紹介していませんが、これらも開発効率を低下させ、バグを生み出す悪しき構造のひとつです。

private int num_events = 3;  // typo: event → event
int code_range = (int) (100 / event_propotion * this.num_events);  // 100, 3の意味不明
int dice = (int) (Math.random() * 6) + 1;  // 6の意味不明

潜んでいる悪魔と弊害

マジックナンバー: 3、100、6などの数値の意味が不明
命名の問題: typoや意味不明な名前（cite_index、figureOut）
弊害:
- コードの意図が理解困難
- 修正時に影響範囲が把握困難
- バグの温床となる

5. リファクタリングの方針

書籍の内容を活用して夥しい量の悪魔を炙り出すことが出来ました。ここからは学んだ内容をもとに悪魔退治を進めていきます。この章では具体的な実装の前に、悪魔を根絶するためのリファクタリング方針を策定します。

生焼けオブジェクトの解消

現状の問題: Gameクラスが初期化されただけでは使い物にならない状態

改善方針:

完全コンストラクタの導入
全クラスで適切な初期化とバリデーションを実装
インスタンス生成時に必要な全ての値を受け取る設計

低凝集の改善

現状の問題: データクラスになっているプレイヤーやマスに関するロジックがeventManagerに散在

改善方針:

データとロジックを統合した値オブジェクトを作成
- Position: プレイヤーの位置を表現する値オブジェクト
- PlayerName: プレイヤー名の値オブジェクト
- WaitTurns: 待機ターン数の値オブジェクト
他のクラスでも、関連するデータとロジックを同じクラス内に配置

可変性の問題解決

現状の問題: オブジェクトの状態が予期せず変更される可能性

改善方針:

全ての値オブジェクトを不変（イミュータブル）に設計
finalキーワードの活用
状態変更時は新インスタンス生成

多重ネストの解消

現状の問題: if-else文による複雑な分岐処理

改善方針:

早期return（ガード節）の適用
EventStrategyインターフェースによるストラテジパターンの導入
各イベント種別のクラス分離（LuckyEvent、UnluckyEvent等）

神クラスの分散

現状の問題: eventManagerが神クラス化し、あらゆる責務を担っている

改善方針:

GameController: ゲーム全体の進行制御に特化
InputHandler: 入力処理を専門化
DisplayManager: 表示処理を独立化
DiceRoller: サイコロ処理の分離
単一責任の原則の適用

共通処理の適切な分離

現状の問題: eventManagerに様々な処理が混在

改善方針:

真に共通的な処理（入力、表示）のみを独立したクラスに分離
特定の処理に関する処理は適切なクラスに配置
汎用的すぎる名前を避け、具体的で意味のある名前を採用

6. リファクタリング後のコード

この章では5章で策定した方針をもとにリファクタリングを実施し、悪魔のいない健全なプログラムへと生まれ変わらせます。以下では、リファクタリング後のプログラムの構造と主な変更点を紹介しています。

値オブジェクト

Position: プレイヤーの位置と移動ロジック
PlayerName: プレイヤー名とバリデーション
WaitTurns: 待機ターン数の管理

状態管理オブジェクト

Player: プレイヤー情報の管理（値オブジェクトを組み合わせ）
Game: ゲーム状態の管理（完全コンストラクタで生焼け解消）
Square: マス情報とイベント実行（ストラテジパターンで委譲）

イベント処理

EventStrategy: イベント処理の共通インターフェース
LuckyEvent, UnluckyEvent等: 各イベントの独立した実装

制御・表示

GameController: ゲーム進行制御のみに責務を限定
InputHandler: 入力処理の専門クラス
DisplayManager: 表示処理の専門クラス
DiceRoller: サイコロ処理の分離

主な変更点

値オブジェクトの導入による低凝集の改善

プリミティブ型の代わりに値オブジェクトを導入してデータを表現し、関連するデータとロジックを集約しました。

// プレイヤーの位置を表現する値オブジェクト
public class Position {
    private final int value;
    
    public Position(int value) {
        if (value < 0) {
            throw new IllegalArgumentException("位置は0以上である必要があります");
        }
        this.value = value;
    }
    
    public Position moveForward(int steps) {
        return new Position(this.value + steps);  // 新インスタンス生成
    }
    
    public Position moveBackward(int steps) {
        // 0未満にならないよう制御して新インスタンス生成
        return new Position(Math.max(0, this.value - steps));
    }
    
    public boolean hasReachedGoal(int goalPosition) {
        // ゴール位置との比較ロジック
        return this.value >= goalPosition;
    }
    
    public int getValue() {
        return value;
    }
}

// プレイヤー名の値オブジェクト
public class PlayerName {
    private final String value;
    
    public PlayerName(String value) {
        // null/空文字/長さ制限のバリデーション処理
        if (value == null || value.trim().isEmpty()) {
            throw new IllegalArgumentException("プレイヤー名は必須です");
        }
        if (value.length() > 10) {
            throw new IllegalArgumentException("プレイヤー名は10文字以内で入力してください");
        }
        this.value = value.trim();
    }
    
    public String getValue() {
        return value;
    }
}

ストラテジパターンによる多重ネスト解消

if-else文による複雑な分岐処理をストラテジパターンで解決しました。

// イベント処理のインターフェース
public interface EventStrategy {
    void execute(Player player, Game game, DiceRoller diceRoller);
    String getEventName();
}

// ラッキーイベントの実装
public class LuckyEvent implements EventStrategy {
    @Override
    public void execute(Player player, Game game, DiceRoller diceRoller) {
        // ラッキーマス到着メッセージ表示
        // サイコロを振ってプレイヤーを前進させる
        // 結果メッセージを表示
    }
    
    @Override
    public String getEventName() {
        return "ラッキーマス";
    }
}

// マスクラスの改善（尋ねるな命じろの適用）
public class Square {
    private final EventStrategy eventStrategy;
    
    public Square(EventStrategy eventStrategy) {
        this.eventStrategy = eventStrategy;
    }
    
    public void executeEvent(Player player, Game game, DiceRoller diceRoller) {
        eventStrategy.execute(player, game, diceRoller);  // 処理を委譲
    }
}

完全コンストラクタによる生焼けオブジェクト解消

全クラスで適切な初期化を実現しました。

// 改善されたPlayerクラス
public class Player {
    private final PlayerName name;
    private Position position;
    private WaitTurns waitTurns;
    
    public Player(PlayerName name, Position initialPosition) {
        if (name == null || initialPosition == null) {
            throw new IllegalArgumentException("名前と初期位置は必須です");
        }
        this.name = name;
        this.position = initialPosition;
        this.waitTurns = new WaitTurns(0);
    }
    
    public void moveForward(int steps) {
        // 位置更新ロジック（新インスタンス生成）
        this.position = position.moveForward(steps);
    }
    
    public boolean canMove() {
        // 移動可能判定ロジック
        return waitTurns.canMove();
    }
    
    // Getters
    public PlayerName getName() { return name; }
    public Position getPosition() { return position; }
}

// 改善されたGameクラス
public class Game {
    private final List<Player> players;
    private final Board board;
    private final int goalPosition;
    private int currentTurn;
    private boolean isGameActive;
    
    public Game(List<Player> players, Board board) {
        // 必須パラメータのバリデーション
        if (players == null || players.isEmpty()) {
            throw new IllegalArgumentException("プレイヤーが必要です");
        }
        if (board == null) {
            throw new IllegalArgumentException("ボードが必要です");
        }
        // 全インスタンス変数を適切に初期化
        this.players = new ArrayList<>(players);
        this.board = board;
        this.goalPosition = board.getSize();
        this.currentTurn = 0;
        this.isGameActive = true;
    }
}

責任の分散による神クラス解消

eventManagerを複数の専門クラスに分散しました。

// ゲーム進行制御に特化
public class GameController {
    private final InputHandler inputHandler;
    private final DisplayManager displayManager;
    private final DiceRoller diceRoller;
    
    public GameController() {
        this.inputHandler = new InputHandler();
        this.displayManager = new DisplayManager();
        this.diceRoller = new DiceRoller();
    }
    
    public void playTurn(Game game) {
        // ターン開始処理
        game.nextTurn();
        displayManager.showTurnStart(game.getCurrentTurn());
        
        // 各プレイヤーの行動処理ループ
        for (Player player : game.getPlayers()) {
            // 待機判定とスキップ処理
            if (!player.canMove()) {
                displayManager.showPlayerWaiting(player);
                player.decreaseWaitTurns();
                continue;
            }
            
            // プレイヤーターン実行
            playPlayerTurn(game, player);
            
            // ゴール判定とゲーム終了制御
            if (player.hasReachedGoal(game.getGoalPosition())) {
                game.endGame();
                break;
            }
        }
        
        // ゲーム状態表示
        displayManager.showGameStatus(game);
    }
}

// 入力処理専門
public class InputHandler {
    private final Scanner scanner;
    
    public InputHandler() {
        this.scanner = new Scanner(System.in);
    }
    
    public PlayerName inputPlayerName() {
        // プロンプト表示と入力受付
        System.out.print("名前を入力してください(全角10文字以内）\n>>>");
        String name = scanner.nextLine();
        // 値オブジェクトに変換（バリデーション委譲）
        return new PlayerName(name);
    }
}

7. リファクタリングの効果確認：仕様追加

６章で私のコードに潜む悪魔は焼き払われ、清きコードへと生まれ変わったことと思います。しかし、果たして本当に悪魔退治に意味があったのでしょうか？この章では既存のゲームに新しい仕様を追加し、リファクタリング前後のコードで仕様変更への対応内容がどのように変化したか確かめていきます。

はじめに、既存のゲームの仕様をおさらいしましょう。

ゲーム進行フロー

プレイヤーの数と名前を最初に入力する

各プレイヤーは毎ターン、エンターキーを押してサイコロを振り、振った数だけマスを進める

到着したマスがイベントマスだった場合、プレイヤーに対してイベントが発生する

プレイヤーが一人でもゴールすれば終了

イベントの種類

ラッキーマス: もう一度サイコロを振り、出た目の数先に進む

ざんねんマス: もう一度サイコロを振り、出た目の数後に戻る

超ざんねんマス: もう一度サイコロを振り、出た目の数ターンがスキップされる

このゲームに、新しいイベントとして「おじゃましマス: 一番ゴールに近いプレイヤーと同じ位置まで移動する」を追加します。以下でリファクタリング前後のコードそれぞれへ仕様追加を行い、必要な対応内容を比較します。

リファクタリング前のコードへの仕様追加

リファクタリング前のコードでは、すべてのイベント処理がeventManagerクラスのjudgeEvent()メソッドに集約されています。ここに新しいイベントを追加するには既存メソッドに条件分岐を追加し、judgeEvent()メソッドをより肥大化させる必要があります。

変更が必要な箇所

eventManagerクラスの大幅修正
judgeEvent()メソッドへの条件分岐追加
イベント数の変更

具体的な変更内容

public void judgeevent(Game g, Player p) {
    Square current_cite = g.field.get(p.cite_index);
    if (current_cite.event_code == 1) {
        // 既存処理
    } else if (current_cite.event_code == 2) {
        // 既存処理  
    } else if (current_cite.event_code == 3) {
        // 既存処理
    } else if (current_cite.event_code == 4) {
        // 新規追加：おじゃましマス処理
        // 全プレイヤーから最も進んでいるプレイヤーを特定
        // 複雑な条件分岐とプレイヤー位置調整処理...
    }
}

リファクタリング後の変更

リファクタリング後のコードでは、ストラテジパターンにより各イベントが独立したクラスとして実装されているため、新しいイベントは新しいクラスを作成するだけで追加できます。既存のコードに一切触れる必要がなく、変更の影響を最小限に抑えられます。

変更が必要な箇所

新しいEventStrategyクラスの追加のみ

具体的な変更内容

// 新しいイベントクラスを追加
public class InterfereEvent implements EventStrategy {
    @Override
    public void execute(Player currentPlayer, Game game, DiceRoller diceRoller) {
        // 最も進んでいるプレイヤーを特定
        // 位置比較とプレイヤー移動処理
        // 結果メッセージ表示
    }
}

// 配列に1行追加するだけ
EventStrategy[] events = {
    new NoEvent(),
    new LuckyEvent(),
    new UnluckyEvent(), 
    new SuperUnluckyEvent(),
    new InterfereEvent()  // この1行を追加
};

効果の比較

リファクタリング前のコードはあらゆる処理が一つのクラスに集中していたため、変更すべき箇所を特定するのが非常に困難でした。また、どのコードがどの場所に影響するか想像しづらく、変更によるバグ発生の可能性が判断できない状態でした。対してリファクタリング後のコードは疎結合高凝集と単一責任の原則が意識されており、仕様変更・追加が容易に行える構造に変化していたため、既存コードに一切触れることなく新機能を追加できました。

8. まとめ

この記事では「良いコード/悪いコードで学ぶ設計入門」の内容と、書籍の内容をもとに入社時研修で作成したすごろくゲームをリファクタリングする流れを紹介しました。
リファクタリング前のコードは仕様追加によるバグ混入リスクが高かったのに対し、リファクタリング後のものは仕様追加が用意な構造になっており、新しいクラスを1つ追加するだけで対応が完了しました。

単一責任の原則や尋ねるな命じろといった原則を意識し、疎結合高凝集なコードを実現することで、バグ混入リスクを軽減し開発効率を向上させられます。

最後に

本記事では扱いませんでしたが、この書籍ではクラス設計の土台となるモデリングについてや、実際に既存システムをリファクタリングする際の進め方、設計品質を向上させる開発プロセスの組み方についても言及があります。今後第２段として、こういった内容も記事に起こしていければと思います。

また本書籍の内容についてはもちろん、反対意見もあります。例えば書籍ではデータクラスが一律に悪いもののように書かれていますが、データクラスが有意性を持って実装される場面もあるはずです。
こういった反対意見にも目を通しながら読むことでより学習の意義が高まるかと思います。以下は本書籍の注意点を書いてくれている記事です。書籍と併せてご覧ください。
良いコード／悪いコードで学ぶ設計入門の感想と注意点

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up

「良いコード悪いコードで学ぶ設計入門」を読んで入社時研修で作ったゲームをリファクタリングしてみた

はじめに

目次

1. 書籍の内容紹介：様々な弊害を引き起こす悪魔的なコード

書籍の特徴

主要な悪魔とその対処方法

生焼けオブジェクト

低凝集

可変による意図せぬ影響

多重ネスト

switch文の重複

共通処理クラス（Common、Util）

密結合（神クラス）

密結合（継承）

2. 悪魔へ対処するための基本原則

3. リファクタリング対象: すごろくゲーム

ゲームの内容

主要なクラス構成

4. リファクタリング前のコードに潜む悪魔

Game.java：生焼けオブジェクト

eventManager.java：神クラス

eventManager.java：尋ねるな命じろの違反による密結合

Player.java, Square.java：低凝集とデータクラス

全体：マジックナンバーと命名の問題

5. リファクタリングの方針

生焼けオブジェクトの解消

低凝集の改善

可変性の問題解決

多重ネストの解消

神クラスの分散

共通処理の適切な分離

6. リファクタリング後のコード

主な変更点

7. リファクタリングの効果確認：仕様追加

リファクタリング前のコードへの仕様追加

リファクタリング後の変更

効果の比較

8. まとめ

最後に