LoginSignup
121
119

More than 5 years have passed since last update.

@suzuki-hoge(鈴木 ほげ)in株式会社ミライトデザイン

副作用ってなんだ？　〜楽に小さく単体テストをしよう〜

Last updated at Posted at 2017-08-07

副作用ってなんだ？

プログラミングにおける副作用（ふくさよう）とは、ある機能がコンピュータの(論理的な)状態を変化させ、
それ以降で得られる結果に影響を与えることをいう。代表的な例は変数への値の代入である。

wikipedia より

つまり？

（ざっくり言うと）同じ様に呼び出しても同じ結果が返ってくるとは限らない処理のことです

何が悪いの？

単体テストがしづらい！！
（以後、この記事ではJUnitの様なテストフレームワークによるソースコードの自動テストを単体テストと言います）

他にも色々面倒を引き起こします

  • テスト環境で評価するのが面倒だったり出来なかったりすることが多い
  • 開発コストがかさむ
  • 改修範囲の特定が難しかったり、切り分けが出来なかったりする

サンプル見せてよ

このクラスを単体テストしたいと思ったけど無理っぽいぞどうしよう、という想定です

Greet.java
@AllArgsConstructor
public class Greet {
    String first;
    String last;
    String gender;

    public void greet() {
        System.out.println(
                getFirstMessage() + forFullName() + getForGender()
        );
    }

    private String getFirstMessage() {
        int hour = LocalDateTime.now().getHour();
        if (6 <= hour && hour < 12) {
            return "おはよう ";
        } else if (12 <= hour && hour < 18) {
            return "こんにちは ";
        } else {
            return "おやすみ ";
        }
    }

    private String getForGender() {
        if (gender.equals("M")) {
            return " くん";
        } else {
            return " ちゃん";
        }
    }

    private String forFullName() {
        return first + " " + last;
    }
}
Main.java
public class Main {
    public static void main(String[] args) {
        Greet greet = new Greet("やまだ", "たかし", "M");
        greet.greet();
    }
}

悪いのはどこ？

  • 保持している変数（以後フィールドと言う）がいつでも変更出来る
  • 現在日時の生成がある
  • 標準出力がある
  • いずれのprivateメソッドも、結果がフィールドに基づいて決まる

これらの理由があるために、僕はこのGreetは副作用まみれだと感じます

副作用の例を教えて

同じ様に呼び出しても同じ結果が返ってくるとは限らない処理のこと

例えばgreet.greet();と同じ様に実行しても、実行した時間帯によって結果が異なります

なんと表示されるでしょうか？
僕にはこれがいつ実行されるかがわかりません！

また、他には例えば以下の様にすると2行目と4行目のコードは全く同じですが、結果は異なります

Main.java
Greet greet = new Greet("やまだ", "たかし", "M");
greet.greet();
greet.gender = 'F';
greet.greet();

それがテスト出来ないにどう関係するの？

例えば朝以下のテストを書いたとして、それが夜失敗するのは明らかです

greet.greet() == 'こんにちは やまだ たかし くん'

それに、そもそもgreet()はvoidです
標準出力をしたら満足して何も返してくれないので、値比較をする機会すら与えてはくれません

（当然、厳密にはできなくないですが、現在時刻のハックと標準出力のハックが必要です）

じゃあ直してみてよ

当然直します
（あくまで個人的にはですが、）こんなコードは許容できません

Greetを見て、何をしているか整理してみましょう

やってることは5つでしょうか

  1. 時間によって最初の単語を変える
  2. 性別によって最後の単語を変える
  3. 姓と名は半角で結合する
  4. それら全てを半角で結合する
  5. 標準出力する

なによりもまずは時間と出力

単体テストをするとなったときに特に問題なのが、1 と 5 の実現方法です

土曜日だけキャンペーンを表示するとか、夜になったら電源を落とすとか、月末月初のバッチ処理とか、連携先会社の業務時間内か判定するとか、時刻判定っていたるところに出てくるはずなんです
（レイヤー構成やコンポーネントの設計にも依りますが、）原則として、時間は判定ロジックの外から渡す様にするべきです
今回の例だとMainから時間を渡さなければGreetのテストはできません

それからもう一つ、組み立てた結果を標準出力されてしまうとテストが出来ません
ですので、ロジックの結果は返却しなければなりません

直してみましょう

Greet.java
@AllArgsConstructor
public class Greet {
    String first;
    String last;
    String gender;
    LocalDateTime now; // 自分で生成せず、もらう

    public String greet() {
        return getFirstMessage() + forFullName() + getForGender(); // 返す
    }

    private String getFirstMessage() {
        int hour = now.getHour();
        if (6 <= hour && hour < 12) {
            return "おはよう ";
        } else if (12 <= hour && hour < 18) {
            return "こんにちは ";
        } else {
            return "おやすみ ";
        }
    }

    private String getForGender() {
        if (gender.equals("M")) {
            return " くん";
        } else {
            return " ちゃん";
        }
    }

    private String forFullName() {
        return first + " " + last;
    }
}

上の２つを直すと、「仮に今X時だとしたら、〜〜〜と返ってくるはず」というテストが書ける様になります

GreetTest.groovy
class GreetTest extends Specification {
    def test_1() {
        setup:
        def greet = new Greet("やまだ", "たかし", "M", LocalDateTime.of(2017, 7, 18, 12, 30, 00))

        expect:
        greet.greet() == 'こんにちは やまだ たかし くん'
    }

    def test_2() {
        setup:
        def greet = new Greet("やまだ", "たかし", "M", LocalDateTime.of(2017, 7, 18, 22, 30, 00))

        expect:
        greet.greet() == 'おやすみ やまだ たかし くん'
    }
}

フィールドアクセス

ついでに、privateメソッドがメソッド内でフィールドにアクセスしてる点を直します
ここは必ずではありませんし、むしろオブジェクト指向的には正しいのかも知れませんが、僕は必ず直した方の例のコードを普段書きます

直すメリットとしては、関連する値がはっきりすることと、private内でフィールドの状態更新されている可能性を潰すためです
（あとで例を示しますが、private staticメソッドにしてしまい、フィールドアクセスを出来なくします）

Greet.java
@AllArgsConstructor
public class Greet {
    String first;
    String last;
    String gender;
    LocalDateTime now;

    public String greet() {
        return getFirstMessage(now) + forFullName(first, last) + getForGender(gender);
    }

    private String getFirstMessage(LocalDateTime now) {      // 引数でもらった値しか使わない
        int hour = now.getHour();
        if (6 <= hour && hour < 12) {
            return "おはよう ";
        } else if (12 <= hour && hour < 18) {
            return "こんにちは ";
        } else {
            return "おやすみ ";
        }
    }

    private String getForGender(String gender) {             // 同様
        if (gender.equals("M")) {
            return " くん";
        } else {
            return " ちゃん";
        }
    }

    private String forFullName(String first, String last) {  // 同様
        return first + " " + last;
    }
}

それぞれのprivateメソッドが何の値にのみ依存しているかがわかりやすくなりました
ところで、それぞれのprivateメソッドは引数にのみ依存して結果を返します
つまり、privateメソッド部分の副作用はなくなったのです！

直したprivateメソッドは一箇所もフィールドアクセスをしていません
ですので、staticメソッドにする事が可能です

staticが付いていれば、thisが使えなくなるので「計算したついでになんかのフラグも立てちゃう」
みたいなフィールドの更新がされないことが保証されます

static（静的）という単語の意味を考えれば、状態に応じて動的に結果がかわらないのですから、安心する感じがなんとなくわかるでしょうか

（static化は下の改善と併わせて記載します）

静的化

さて、最後に仕上げです
greet()だけはフィールドに依存していますが、別にフィールドに値を蓄えておく必要はなくなったのです

Greet.java
public class Greet {

    // フィールドがなくなった

    public static String greet(String first, String last, String gender, LocalDateTime now) { // ここが全て引数になった
        return getFirstMessage(now) + forFullName(first, last) + getForGender(gender);
    }

    private static String getFirstMessage(LocalDateTime now) {
        int hour = now.getHour();
        if (6 <= hour && hour < 12) {
            return "おはよう ";
        } else if (12 <= hour && hour < 18) {
            return "こんにちは ";
        } else {
            return "おやすみ ";
        }
    }

    private static String getForGender(String gender) {
        if (gender.equals("M")) {
            return " くん";
        } else {
            return " ちゃん";
        }
    }

    private static String forFullName(String first, String last) {
        return first + " " + last;
    }

    // 全てのメソッドが static になった
}

ただし、newをする必要がなくなったので、呼び出し元の変更も必要です

GreetTest.groovy
class GreetTest extends Specification {
    def test_1() {
        expect:
        Greet.greet("やまだ", "たかし", "M", LocalDateTime.of(2017, 7, 18, 12, 30, 00)) == 'こんにちは やまだ たかし くん'
    }

    def test_2() {
        expect:
        Greet.greet("やまだ", "たかし", "M", LocalDateTime.of(2017, 7, 18, 22, 30, 00)) == 'おやすみ やまだ たかし くん'
    }
}

これでこのテストは何時に実行しても通るし、この引数で呼べばどうやっても別の結果を返させることは出来なくなりました！

オブジェクト指向に反している！？

確かに、そんな気もします...
そもそも僕が副作用排除に魅力を感じるのは関数型言語の影響が大きいです

ですがそれだけの意見だと感想にすらならないので、ちょっと考えてみました

オブジェクト指向でコードを書いているとクラスには3種類ある？

えぇ、今思いついたテキトー発言です

  1. 値を持たず、受け取った値で判定したり計算したりするクラス
  2. 値を持ち、自身の値に基づいて振る舞うクラス
  3. それらを扱うクラス

ひとつずつ見てみましょう

一番上のクラスは言わずもがな、先ほど直したGreetクラスです
こいつは自分の値を持っておらず、引数だけに依存しています

二番目のクラスはなんでしょう
ここが一番オブジェクト指向的にしっくり来ると思いますが、「人名」や「性別」の様な「モノ」を現すクラスです

ちょっとさっきのお題に登場させてみましょうか

First.java
@AllArgsConstructor
public class First {
    @Getter
    private final String value;
}
Last.java
@AllArgsConstructor
public class Last {
    @Getter
    private final String value;
}
Gender.java
public enum Gender {
    M, F
}
User.java
@AllArgsConstructor
public class User {
    private final First first;
    private final Last last;
    private final Gender gender;

    public String asFullNameString() {
        return first.getValue() + " " + last.getValue();
    }

    public boolean isM() {
        return gender == Gender.M;
    }
}

Userという「人」を現すクラスを作り、性別判定やフルネーム連結はそっちに移動してみました

これが実は結構良い責務分割だと感じています
理由としては、Greet「性別はMと比較する」とか「姓と名は半角で繋ぐ」とかの部分が、実は挨拶とは関係ないことが分かったからです
挨拶に求められていることは「時間と性別によって姓名をちょっと加工する」だけで、「男性がM」だとか「半角繋ぎ」とかは「人の定義」に準ずる処理だからです
（当然仕様や設計によりそうでない場合もあります）

そして当然Userの単体テストです、これは絶対です

「男性判定」と「姓名連結」をUserに移動してUserの単体テストをしておけば、Greetのテストは時間帯判断とくんちゃん出しわけだけになります

Greet.java
public class Greet {
    public static String greet(User user, LocalDateTime now) {
        return getFirstMessage(now) + user.asFullNameString() + getForGender(user);
    }

    private static String getFirstMessage(LocalDateTime now) {
        int hour = now.getHour();
        if (6 <= hour && hour < 12) {
            return "おはよう ";
        } else if (12 <= hour && hour < 18) {
            return "こんにちは ";
        } else {
            return "おやすみ ";
        }
    }

    private static String getForGender(User user) {
        if (user.isM()) {
            return " くん";
        } else {
            return " ちゃん";
        }
    }
}

Greetがまたすっきりしましたね

  1. それらを扱うクラス

そして最後に三番目ですが、これは今回だとMainが相当しますね

Userを作ってGreetを使って計算させています（コードは割愛します）

3種類のまとめ

言い換えるとこんな感じになるのではないでしょうか

  1. ロジック
  2. モノ
  3. 処理

ロジックは基本的にstaticで状態には依存してはならない

モノは値を保持しそれに基づいて振る舞うが、値自体やロジック自体は隠蔽する（カプセル化に相当するのかな）

その２つを作用させ合う処理層がある、副作用は処理層でのみ許される
（副作用はなくせないのでここでやる　このクラスのテストはモック等を使って行うが、それはまたいずれ）

余談？ ロジックとモノの境界について

「男性ならくん、女性ならちゃん」ってのはGreetの決めることなのか？Userじゃあねーのか？

「Userが半角スペースで結合したけど、メール送るときは全角で結合しないといけない」んだよどーすんだ？

とか思うかもしれないです
僕はちょっと思いました

これって「ロジック？」「モノ？」って思った場合は、「ロジック専用のモノ」を作ってしまうというのに最近は落ち着きました

つまり、「Greet用User」を作ってしまい、「くんちゃん」と「半角結合」はその挨拶特化ユーザクラスに書きます
メールの時はまた違うなら、メール特化ユーザクラスを別に設けます

これがさらなる責務分割というか、境界をはっきりさせるというか、そんな感じだと思います
こうしておけばクラスは増えますが、そんなにテスト項目は増えない気がしますし、何より依存度が下がります

「メールの仕様変更に伴い挨拶の再評価」とかをしなくてよくなるのです！

まとめ

副作用の例

この辺を見かけたら要注意です！！

  • 何が得られるかわからない
    • 現在時刻取得
    • ファイルから読み込み
    • DBから読み込み
    • 環境変数の参照
    • グローバル変数の参照
    • 乱数生成
  • 書いて終わってしまうと何を書いたかテスト出来ない
    • ファイル書き込み
    • DBへ書き込み
    • 標準出力

参照するときは「ロジック、モノ」の外で行い、「ロジック、モノ」はそれに依存してはいけない
組み立てた値はその場で書いてしまわず、一度処理層に返却し、また処理層に書き込ませる

簡単判定ガイド

端的に言うと、「ロジック、モノ」のテストで以下が必要になったら副作用が混じっている

  • モック
  • テストデータ
    • 通信結果のJsonやデータベースのダミーデータ挿入等
  • 環境変数設定
  • 現在時刻の改竄
  • OS判定
  • 「ロジック、モノ」におけるvoidメソッド
    • 何も返さないと言う事は、内部の状態更新やPOST通信、ファイル書き込み等をしているという事

これらを徹底していくと、評価に苦しむ様なコードが減り、単体テストが増え、開発ペースも向上するのではないでしょうか

それではまた

121
119
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
121
119