0
0

ユニットテストの価値を高めるリファクタリング

Posted at

対象読者

  • テストコードを書いた経験のある方
  • テスト対象コードが複雑で、テストに苦労した経験のある方

本記事のゴール

  • リファクタリングするべきコードの種類を把握する
  • テストする価値が高いコードを把握する
  • どのようにしてリファクタリングするべきかを把握する

リファクタリングが必要なコードを識別する

テストコードを楽に書くためには、テスト対象コードのリファクタリングが必要不可欠です。
本記事では、リファクタリングが必要なコードの識別方法と、そのリファクタリング方法について解説します。

本記事では、テスト対象コードとテストコードを区別するために、
テスト対象コード = 「プロダクションコード」と表現します。
また、本記事で言う「テスト」はテストコードを用いたユニットテストを想定しています

4種類のプロダクションコード

ほとんどの場合、プロダクションコードは以下2つの視点で分類します。

  • コードの複雑さ、ドメインにおける重要性
  • 協力者オブジェクトの数

コードの複雑さ とは、プロダクションコードに含まれている条件分岐の数で測定します。
条件分岐が多いほど、そのクラスは複雑なコードであると言えます。

一方、ドメインにおける重要性 とは、そのコードがシステムの業務領域においてどのくらい重要なのか を示すものです。
決済アプリを例にしてみます。
「夏季期間に決済を行った場合のみ、ポイント付与率が1.5%になる」 という仕様があるとします。
これをコードで表現すると、以下になります。

SummerPay.java
Calendar now = Calendar.getInstance();
if(isSummerSeason(now)){
// ポイント付与率1.5%の適用(コードの詳細は省略
}

SummerPayクラスで定義された上記のコードは、「夏季期間に決済を行った場合のみ、ポイント付与率が1.5%になる」 という仕様に直結するロジックであり、ドメインにおける重要性が高い と言えます。

つまり、システムの仕様やユーザーの求めていることに対して、コードがどの程度結びついているかがドメインにおける重要性となります。

次に協力者オブジェクトの数 についてですが、これはプロダクションコードが外部プロセスに依存している数 を指します。
一般的なアプリケーションを例にすると、Web APIやデータベース等のシステム外のモノに依存しています。
【例】

  1. Web APIを用いてデータを取得する
  2. 取得したデータを加工する
  3. データをデータベースに登録する

上記の処理のうち、「Web APIを用いてデータを取得する」と、「データをデータベースに登録する」処理は、専用のクラスを用意するのが一般的です。

  • Web APIをリクエストするクラス
  • データベースにアクセスするクラス

つまり、プロダクションコードでは上記のクラスの協力を得て処理を実現していることになります。
これらのクラスが、協力者オブジェクトとしてカウントされます。

ここでいう協力者オブジェクトには、静的(static)メソッドも含まれることに注意してください。
例えば、以下のコードではPathsクラスのget(static)メソッドを経由してファイルパスを取得しています。
Path path = Paths.get("hoge/fuga/foo.txt");
これはシステム外のプロセスにアクセスしていると言えるので、協力者オブジェクトとしてカウントする必要があります。

  • コードの複雑さ、ドメインにおける重要性
  • 協力者オブジェクトの数

この2つの観点で見ることで、プロダクションコードを以下4種類に分類できます。

コードの複雑さ/ドメインにおける重要性 協力者オブジェクトの数 プロダクションコードの種類
低い 少ない 取るに足らないコード
高い 少ない ドメイン・モデル/アルゴリズム
低い 多い コントローラー
高い 多い 過度に複雑なコード

それぞれのプロダクションコードの種類について見ていきましょう。

  • 取るに足らないコード
    SetterやGetter等のロジックを含まないコードが該当します。
    コードも単純で、協力者オブジェクトを持たないコードです。

  • ドメイン・モデル/アルゴリズム
    システムの仕様に直結するようなロジックや、複雑な条件分岐を持つコードが該当します。反面、持っている協力者オブジェクトの数は少ないです。

  • コントローラー
    複数の協力者オブジェクト(またはドメインモデル・アルゴリズムなどを扱うコンポーネント)を持ち、各協力者オブジェクト間の連携を行います。
    反面、複雑な条件分岐やロジックは持ちません。あくまで複数の協力者オブジェクトを操作する指揮官の役割を持ちます。

  • 過度に複雑なコード
    複数の協力者オブジェクトを持ちながら、複雑なロジックを持つコードです。
    テストするのが非常に大変なコードです。

ドメイン・モデル/アルゴリズム,コントローラーを野球で例えてみる

ドメイン・モデル/アルゴリズム = 野球選手
コントローラー = 監督
になります。

ドメイン・モデル/アルゴリズム を野球選手で例えてみます。
野球選手には、様々な特徴を持つ選手がいます。

  • 守備が得意な選手
  • ピッチングが得意な選手
  • バッティングが得意な選手

みんな、何かしら得意な技術を持っています。

野球選手たち.png

しかし、それ以外には関心を持ちません。
例として、ピッチャーはバッターを打ち取ることだけに専念しますが、
バッティングにはあまり注力しません。
ピッチャーの仕事は、バッターを抑えることでだからです。
点を取るのは、バッターに任せればよいのです。
(大谷翔平選手は例外です。笑)

監督(コントローラー)についても見てみましょう。
監督の仕事は、ズバリ采配です。
誰を起用し、どのような作戦を立てるか を考え選手に指示を出すのが仕事です。
しかし、監督自身が選手として試合に出ることはありません。
監督の仕事はあくまで、選手に指示を出すことです。
自ら手を動かすことはありません。
監督.png

監督はたくさんの選手に指示を出す。選手は指示に従って自分の仕事をこなす

ドメイン・モデル/アルゴリズム、コントローラーにも同じことが言えます。
コントローラーは複数の協力者オブジェクトを抱えており、1つの目標を達成するために
協力者オブジェクトに対して指示を出します。
ここで大事なのは、「コントローラーは指揮官に徹する」ことです。
監督自身がプレイヤーとして出場するのは、負担が大きくなるからです。

一方、協力者オブジェクトであるドメインモデル・アルゴリズムでは、
コントローラーから受けた指示に従って、適切な処理を行います。
ここで重要なのは、「指示されたこと以外には関心を持たない」事です。

  • ピッチャーは、バッターを抑えることに専念する
  • バッターは、点を取ることに専念する

自分の仕事に専念することが大事なのです。

リファクタリングするべきは、「過度に複雑なコード」

コードの複雑さ/ドメインにおける重要性 協力者オブジェクトの数 プロダクションコードの種類
低い 少ない 取るに足らないコード
高い 少ない ドメイン・モデル/アルゴリズム
低い 多い コントローラー
高い 多い 過度に複雑なコード

ここで、再度プロダクションコードの分類表を再掲します。
この4種類の中で、リファクタリングするべきなのは、ズバリ「過度に複雑なコードです」
先ほどの野球を例にすると、このコードは 選手兼監督 のようなものです。
背負っている責任が多いんです。

このコードをテストしやすくリファクタリングするには、
ドメイン・モデル/アルゴリズム または、コントローラーのいずれかに分解する必要があります。
システム設計原則っぽく言うと、単一責任原則に従おう という話です。

サンプルコード

ここからは、サンプルコードを用いてどのようにリファクタリングを行うか見ていきましょう。
今回のサンプルコードは、ユーザー管理システムです。
登録されたユーザー情報はデータベースで管理されていると仮定します。
このシステムに対し、「ユーザーのメールアドレスを変更できるようにする」機能の追加を求められていると仮定します。その機能には、以下の条件が設けられています。

【条件】

  1. もし、ユーザーのメールアドレスに自社ドメイン名が含まれている場合、そのユーザーの種類(type)を「従業員」(employee)として登録する。
    それ以外の場合は、「顧客」(customer)として登録する。
  2. 登録されたユーザーの中から従業員となるユーザーの数(従業員数)を管理する。もし、ユーザーの種類が従業員から顧客に変わる場合は、従業員数を減らす。
    逆に、顧客から従業員に変わる場合は、従業員数を増やすようにする。
  3. メールアドレスの変更が完了すると、メッセージ送信を行い、メールアドレスの変更が完了したことを外部システムへ知らせる。
User.java
import java.util.Objects;

public class User {
    private int userId;
    private String email;
    private UserType type;

    /**
     * ユーザーのメールアドレスを変更する
     *
     * @param userId
     * @param newEmail
     */
    public void changeEmail(int userId, String newEmail) {
        // 指定したユーザーのメールアドレスと種類をデータベースから取得する
        Object[] data = Database.getUserById(userId);
        this.userId = userId;
        this.email = (String) data[1];
        this.type = (UserType) data[2];

        if (Objects.equals(this.email, newEmail)) {
            return;
        }

        // 会社のドメイン名と従業員数を取得する
        Object[] companyData = Database.getCompany();
        String companyDomainName = (String) companyData[0];
        int numberOfEmployees = (int) companyData[1];

        String emailDomain = newEmail.split("@")[1];
        boolean isEmailCorporate = emailDomain.equals(companyDomainName);

        // ★条件1:メールアドレスに含まれるドメイン名をもとにユーザーの種類を決める
        UserType newType = isEmailCorporate ? UserType.EMPLOYEE : UserType.CUSTOMER;

        if (this.type != newType) {
            int delta = newType == UserType.EMPLOYEE ? 1 : -1;
            int newNumber = numberOfEmployees + delta;
            // ★条件2:必要に応じて従業員数を更新する
            Database.saveCompany(newNumber);
        }

        this.email = newEmail;
        this.type = newType;

        // ユーザー情報をデータベースに保存する
        Database.saveUser(this);
        // ★条件3:外部システムへメールアドレス変更完了のメッセージを送信する
        MessageBus.sendEmailChangedMessage(this.userId, newEmail);
    }

    // Getters
    public int getUserId() {
        return userId;
    }

    public String getEmail() {
        return email;
    }

    public UserType getType() {
        return type;
    }
}

/**
 * ユーザーの種類
 */
enum UserType {
    /**
     * 顧客
     */
    CUSTOMER,
    /**
     * 従業員
     */
    EMPLOYEE
}

サンプルコードでは簡潔さを優先して、Databaseクラス・MessageBusクラスのコードは省略しています。
また、エラーチェック処理も省略しています。

どこがNGなのか?

Userクラスは、以下の協力者オブジェクトを持っています。

  • データベースへアクセスし、ユーザー情報を取得・登録するクラス(Databeseクラス)
  • 外部システムへメッセージを送信するクラス(MessageBusクラス)

つまり、Userクラスはシステム外とやり取りを行うオブジェクトを持っているため、コントローラー の特徴を持っています。
※厳密に言うと、協力者オブジェクトの数が〇〇以上あれば、コントローラーと分類する ような明示的な境界線はありません。ここは、各プロジェクトで変わってくると思われます。

一方で、メールアドレス変更機能の核となる「ユーザーの種類の決定」と「従業員数の増減」を扱う重要な条件分岐も含んでいます。

条件1
// ★条件1:メールアドレスに含まれるドメイン名をもとにユーザーの種類を決める
UserType newType = isEmailCorporate ? UserType.EMPLOYEE : UserType.CUSTOMER;
条件2
if (this.type != newType) {
    int delta = newType == UserType.EMPLOYEE ? 1 : -1;
    int newNumber = numberOfEmployees + delta;
    // ★条件2:必要に応じて従業員数を更新する
    Database.saveCompany(newNumber);
}

これらの条件分岐は、単純でありながらもメールアドレス変更機能における重要な意思決定を下す部分です。
つまり、ドメインにおける重要性が高いコードになります。
(複雑な分岐は持たないが、機能における重要な部分)

つまり、Userクラスは以下の特徴を持っているため過度に複雑なコードであると言えます。

  • コントローラー
  • ドメイン・モデル/アルゴリズム

この手のコードをテストするのは、テスト準備が非常に大変です。

例えば、条件1(ユーザーの種類を決定する)部分をテストするためには、直前の処理であるデータベースから従業員情報を取得する処理をモックしないといけません。
また、後続処理のデータベース保存、外部システムへのメッセージ送信部分もモック必要です。

「ユーザーの種類を決定する」部分だけをテストしたいのに、それとは関係ない部分に労力を費やす必要があるのです。

テストするべきプロダクションコードの種類は、ズバリ「ドメイン・モデル/アルゴリズム」

プロジェクト内のすべてのコードをテストすることはあまり推奨されていません。
なぜなら、テストコードを書くにあたっての費用対効果が薄くなるからです。

例えば、セッター、ゲッターしか持たないような単純なエンティティクラスをテストしたとしても、バグが見つかる可能性はほぼありません。つまりテストをする意味が無いに等しいです。

システムの品質を担保するためには、重要なコードに絞ってテストを行うことで費用対効果が高まります
そのため、最優先でテストするべきなのはドメイン・モデル/アルゴリズム(システムの仕様に直結するようなロジック・複雑さを持つ)のコードになります。
なぜなら、ソースコード修正時のリグレッションテストを素早く行え、システムの重要な部分の品質を担保しやすくなるためです。

ドメイン・モデル/アルゴリズム 以外のコードは全くテストしなくて良いわけではありません。あくまで、優先度としての話です。
プロジェクトによっては、「コントローラー」や「取るに足らないコード」でも、ある程度のロジックが含まれていればテストする価値はあると思います。

サンプルコードの責務を分担していく

それでは、リファクタリングに入っていきます。
今回のUserクラスは、「過度に複雑なコード」と分類しました。
この種類のコードの問題点は、1つのクラスが複数の責任を抱えているところに問題があります。

そのため、単一責任原則に従い、過度に複雑なコードを以下の種類に分別できるように処理を切り分けます。

  • ドメイン・モデル/アルゴリズム
  • コントローラー

ドメイン・モデル/アルゴリズムに分解したコード

ここでメールアドレス変更機能の条件を、もう一度確認しましょう。

  1. もし、ユーザーのメールアドレスに自社ドメイン名が含まれている場合、そのユーザーの種類(type)を「従業員」(employee)として登録する。
    それ以外の場合は、「顧客」(customer)として登録する。
  2. 登録されたユーザーの中から従業員となるユーザーの数(従業員数)を管理する。もし、ユーザーの種類が従業員から顧客に変わる場合は、従業員数を減らす。

この条件は、メールアドレス変更機能の核となる部分、つまり「ドメインにおける重要性が高い」ことになります。
そのため、この条件1,条件2を行うクラスを用意します。

また、自社ドメイン・従業員数は会社が管理するものなので、以下の処理は会社が担うべき責任と判断できます。
そのため、会社を表現する新しいクラス(Company)を作ります。

  • メールアドレスに自社ドメインが含まれているか?
  • 従業員数の増減処理

またメールアドレス変更機能は、ユーザーに対して行われるものなので、
メールアドレス変更機能(メソッド)は、Userクラスで持つようにします。

上記に従ってリファクタリングを行ったプロダクションコードが以下になります。

User.java
/**
 * ユーザーを表すクラス
 */
public class User {
    private int userId;
    private String email;
    private UserType type;

    public User(int userId, String email, UserType type) {
        this.userId = userId;
        this.email = email;
        this.type = type;
    }

    public int getUserId() {
        return userId;
    }

    public String getEmail() {
        return email;
    }

    public UserType getType() {
        return type;
    }

    /**
     * メールアドレスの変更を行う
     *
     * @param newEmail
     * @param company
     */
    public void changeEmail(String newEmail, Company company) {
        if (email.equals(newEmail))
            return;

        // ★条件1:メールアドレスに含まれるドメイン名をもとにユーザーの種類を決める
        UserType newType = company.isEmailCorporate(newEmail)
                ? UserType.EMPLOYEE
                : UserType.CUSTOMER;

        if (type != newType) {
            // ★条件2:必要に応じて従業員数を更新する
            // 変更したメールアドレスが自社ドメインの場合は従業員数を増やす
            int delta = newType == UserType.EMPLOYEE ? 1 : -1;
            company.changeNumberOfEmployees(delta);
        }

        email = newEmail;
        type = newType;
    }
}
Company.java
/**
 * 会社を表すクラス
 */
public class Company {
    /**
     * 会社のドメイン名
     */
    private String domainName;
    /**
     * 従業員数
     */
    private int numberOfEmployees;

    public Company(String domainName, int numberOfEmployees) {
        this.domainName = domainName;
        this.numberOfEmployees = numberOfEmployees;
    }

    /**
     * 従業員数を更新する
     *
     * @param delta
     */
    public void changeNumberOfEmployees(int delta) {
        // 従業員数の増減結果がマイナスにならないように保護する
        Precondition.requires(numberOfEmployees + delta >= 0);

        numberOfEmployees += delta;
    }

    /**
     * メールアドレスに自社ドメインが含まれているかチェックする
     *
     * @param email
     * @return
     */
    public boolean isEmailCorporate(String email) {
        String emailDomain = email.split("@")[1];
        return emailDomain.equals(domainName);
    }

    public String getDomainName() {
        return domainName;
    }

    public int getNumberOfEmployees() {
        return numberOfEmployees;
    }
}

これにより、複雑なコードからメールアドレス変更機能の核となる部分を切り離すことができました。
Userクラス・Companyクラスは協力者オブジェクトを持っていないため、モックをする必要がなくなりました。
そのため、テスト準備が容易になり、テストを行う費用対効果を高めることができました。
(システムにおける重要な部分を、簡単にテストできるようになった)

コントローラーに分解したコード

メールアドレス変更機能の核となる部分を切り離すことに成功しました。
次は、これらのクラスを呼び出してメールアドレス変更機能の一連の処理を行う「コントローラー」部分をリファクタリングします。
ここで、メールアドレス変更処理の一連の流れを整理しましょう。

  1. 指定したユーザーのメールアドレスと種類をデータベースから取得する
  2. 会社のドメイン名と従業員数を取得する
  3. メールアドレスを変更する
  4. ユーザー情報・会社情報をデータベースへ保存する
  5. 外部システムへ、メールアドレス変更完了のメッセージを送信する

これら一連の流れを行うコントローラークラスを作ります。
「3.メールアドレスを変更する」処理は、Userクラスに定義した changeEmail()メソッドを呼べばOKです。

リファクタリングしたプロダクションコードがこちらになります。

UserController.java
/**
 * メールアドレス変更を行う起点となるコントローラークラス
 */
public class UserController {
    private final Database database = new Database();
    private final MessageBus messageBus = new MessageBus();

    /**
     * メールアドレスを変更する
     *
     * @param userId
     * @param newEmail
     */
    public void changeEmail(int userId, String newEmail) {
        // 指定したユーザーのメールアドレスと種類をデータベースから取得する
        Object[] userData = database.getUserById(userId);
        User user = UserFactory.create(userData);

        // 会社のドメイン名と従業員数を取得する
        Object[] companyData = database.getCompany();
        Company company = CompanyFactory.create(companyData);

        // メールアドレスを変更する
        user.changeEmail(newEmail, company);

        // ユーザー情報・会社情報をデータベースへ保存する
        database.saveUser(user);
        database.saveCompany(company);

        // ★条件3:外部システムへメールアドレス変更完了のメッセージを送信する
        messageBus.sendEmailChangedMessage(userId, newEmail);
    }
}

リファクタリング後のプロダクションコードで注目したいのが、UserControllerクラスは協力者オブジェクトに対して指示出しをしているだけという事です。
自分自身は複雑なロジックを持っておらず、「メールアドレスを変更する」使命を遂行するために、様々な協力者オブジェクトに指示を出すことに徹しています。

複雑なロジックは持っていないため、テストをする際は協力者オブジェクトをモックすることだけに注力すれば良いのです。

また、リファクタリング前のコードでは、ユーザー情報・会社情報を変換する処理がありました。

// 指定したユーザーのメールアドレスと種類をデータベースから取得する
Object[] data = Database.getUserById(userId);
this.userId = userId;
this.email = (String) data[1];
this.type = (UserType) data[2];
// 会社のドメイン名と従業員数を取得する
Object[] companyData = Database.getCompany();
String companyDomainName = (String) companyData[0];
int numberOfEmployees = (int) companyData[1];

この処理も、変換クラス(Factoryクラス)を用意することで、UserControllerクラス自身が変換ロジックを持たないようにしています。
(Factoryクラスのコードは、次の「取るに足らないコード」の章で確認します。)

取るに足らないコード に分解したコード

前の章でも少し書きましたが、ユーザー情報・会社情報を変換する処理を行うファクトリークラスを作りました。
このクラスは協力者オブジェクトも持っておらず、複雑な条件分岐もないため テストは簡単にできます。

UserFactory.java
/**
 * ユーザーオブジェクトを作成するファクトリークラス
 */
public class UserFactory {
    public static User create(Object[] data) {
        // 不正な引数が渡されてないかチェックする
        Precondition.requires(data.length >= 3);

        int id = (int) data[0];
        String email = (String) data[1];
        UserType type = (UserType) data[2];

        return new User(id, email, type);
    }
}
CompanyFactory.java
/**
 * 会社のオブジェクトを作成するファクトリークラス
 */
public class CompanyFactory {
    public static Company create(Object[] data) {
        Precondition.requires(data.length >= 2);

        // ドメイン名
        String domainName = (String) data[0];
        // 従業員数
        int numberOfEmployees = (int) data[1];

        return new Company(domainName, numberOfEmployees);
    }
}

UserType.java
/**
 * ユーザーの種類
 */
public enum UserType {
    /**
     * 顧客
     */
    CUSTOMER(1),
    /**
     * 従業員
     */
    EMPLOYEE(2);

    private final int value;

    UserType(int value) {
        this.value = value;
    }

    public int getValue() {
        return value;
    }
}

最後に、エラー防止用のクラスを追加しました。
引数の真偽値がfalseの場合、例外をthrowするようになっています。

Precondition.java
/**
 * 不正チェック用のクラス
 */
public class Precondition {
    public static void requires(boolean condition) {
        if (!condition) {
            throw new IllegalArgumentException("Precondition failed");
        }
    }
}
使い方例
// 不正な引数が渡されてないかチェックする
Precondition.requires(data.length >= 3);

こうすることで、エラーチェックをPreconditionクラスに任せることができ、
他クラスのコードが簡潔に書けるようになりました。

リファクタリング後のプロダクションコードの種類を再確認

最後に、リファクタリングを行ったコードが、それぞれどのプロダクションコードに分類されるか確認してみましょう。

コードの複雑さ/ドメインにおける重要性 協力者オブジェクトの数 プロダクションコードの種類
低い 少ない 取るに足らないコード
高い 少ない ドメイン・モデル/アルゴリズム
低い 多い コントローラー
高い 多い 過度に複雑なコード
  • ドメイン・モデル/アルゴリズム

    • User クラス
    • Company クラス
  • コントローラー

    • UserController クラス
  • 取るに足らないコード

    • UserFactory クラス
    • CompanyFactory クラス
    • UserType クラス
    • Precondition クラス

リファクタリングを行うことで、過度に複雑なコードがなくなりました。
また、ドメインにおける重要性の高い(最もテストする価値がある)Userクラス・Companyクラスは協力者オブジェクトを持たないため、モックが不要となりテストが簡単に行えるようになりました。
少ない労力でシステムの重要な部分をテストすることができるようになった)

今回行ったリファクタリングは、MVCモデルへの分解 とも言えます。

  • M(モデル):ロジックを担当する部分
  • V(ビュー):画面を担当する部分
  • C(コントローラー):モデルとビューの間を取り持って、制御を担当する部分

今回のリファクタリングポイントは、複雑な処理をモデルとコントローラーに切り分けることです。

参考文献

単体テストの考え方/使い方
7章:単体テストの価値を高めるリファクタリング

記事で扱っているサンプルコードは、本書のサンプルコードをJavaに書き換えたものです。
元ソースはこちら

0
0
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
0
0