« [コラム] リファクタリングはどっちへ進むべきだろう? (続々) | トップページ | [コラム] F# で NUnit するには »

2010年10月25日 (月)

[コラム] リファクタリングはどっちへ進むべきだろう? (了)

前回、 FizzBuzz は、4つの小さな仕様に分割できることを述べました。

繰り返すと、 FizzBuzz は正の整数すべてを受け付け、 それらを 4つに分類して、 それぞれに対して処理をします。 分類するところまでは分割できませんが、 分類できてしまえば、 そのあとは 4つ別々に処理することが可能でした。 小さな 4つの独立した仕様に分解できて、 それぞれを個別に考えればよかったのでした。

 5の倍数である5の倍数でない
3の倍数である "Fizz Buzz" と言う "Fizz" と言う
3の倍数でない "Buzz" と言う その数を言う

FizzBuzz では、 4つのそれぞれの処理はごく短いものなので、 ここまで考える必要は無いのですが、 仮にそれぞれがけっこう大きな処理 (100行とか 1000行とか) だとしましょう。 そんな長い処理を 4つも、 ひとつのメソッドに詰め込んだのでは、 まずまちがいなくとても理解しづらいコードになってしまいますから、 4つの処理をそれぞれ別のメソッドとして切り出すことでしょう。

前回提示した SayByDecidionTable() メソッドから、 そのように 4つのメソッドを切り出すと、 次のようになります。
※ 3と5の公倍数のとき、 "FizzBuzz" ではなく本来の "Fizz Buzz" にしてあります。

public static string SayByDecidionTable(int n)
{
    if (n <= 0)
        throw new ArgumentOutOfRangeException();

    bool is3の倍数 = (n % 3 == 0);
    bool is5の倍数 = (n % 5 == 0);

    if (is3の倍数)
    {
        if (is5の倍数)
            return SayAt3と5の公倍数();
        else
            return SayAt3の倍数();
    }
    else
    {
        if (is5の倍数)
            return SayAt5の倍数();
        else
            return SayAtそれら以外(n);
    }
}

private static string SayAt3と5の公倍数()
{
    return "Fizz Buzz";
}

private static string SayAt3の倍数()
{
    return "Fizz"; //ここが 1000行くらいあると思いねえ
}

private static string SayAt5の倍数()
{
    return "Buzz"; //ここにも 1000行くらいあると…
}

private static string SayAtそれら以外(int n)
{
    return n.ToString(); //同じく…
}

※ くどいようですが、 これはそれぞれの SayAt…() メソッドの中身が大きい場合を仮定しての話です。 本来の FizzBuzz でこんなことをする必要は無いでしょう。

ここで、 SayAt3と5の公倍数() メソッドに次のようなコードが書いてあったとしたら、 どうでしょうか?

private static string SayAt3と5の公倍数()
{
    return string.Concat(SayAt3の倍数(), " ", SayAt5の倍数());
}

理解しやすいですか?
「これ理解するには、 SayAt3の倍数() と SayAt5の倍数() の中身を両方読まんといかんじゃないか! それって、 ほぼ全部読め ってことだろ!?
…そう考えるだろうと思います。

このメソッドは、 4つに分割した仕様のひとつ 「(3の倍数かつ5の倍数のときは) "Fizz Buzz" と言う」 だけを実装すればよいだけ、 のはずです。 なのに、 3の倍数のときの結果と 5の倍数のときの結果が必要になるとは!?
せっかく 4つに分割した仕様が、 影響しあっています。 干渉しています。
このメソッドを書くとき、 あるいは後で読むとき、 SayAt3の倍数() メソッドと SayAt5の倍数() メソッドについても同時に考えねばなりません。 仕様の 4分の 1 にあたる実装を理解するために、 コードの 4分の 3 を読まねばなりません。 大きな仕様を、 独立した小さな仕様に分割することができたのに、 そのメリットを捨ててしまっているのです。
もちろん仕様がそうなっているのなら、 干渉しあうコードになるのは当然なわけですが、 そんなことは FizzBuzz の仕様に書かれていませんよね。 ここの仕様は 「"Fizz Buzz" と言う」 となっているだけです。 わざわざ仕様間の干渉を実装して理解しにくくする理由はありません。

次のようにして、 3の倍数のときに戻す文字列に "Fizz" を追加し、 また 5の倍数の時に "Buzz" を付け加えるというコードも、 上で説明したコードと同類です。 ちょっとわかりにくいのですが、 3の倍数のときの結果と、 5の倍数の時の結果が合わさって、 3と5の公倍数のときの結果になっています。

    string result = "";

    if (is3の倍数)
        result += "Fizz";
       
    if (is5の倍数)
    {
        if (string.IsNullOrEmpty(result))
            result += "Buzz";
        else
            result += " Buzz";
    }

    if (is3の倍数 && is5の倍数)
        ; // (処理済み。何もしない。)

    if (!is3の倍数 && !is5の倍数) //ここは if(result.Length == 0) で代用できる
        result = n.ToString();

    return result;

 

もう一度言います。

private static string SayAt3と5の公倍数()
{
    return string.Concat(SayAt3の倍数(), " ", SayAt5の倍数());
}

このように仕様を 「混ぜて」、 話をややこしくする理由はありません。
シンプルに、

private static string SayAt3と5の公倍数()
{
    return "Fizz Buzz";
}

と書くことで、 仕様を満たすことができます。

リファクタリングはシンプルなコードを目指しますが、 それは 「短いコード」 のことではありません。 一言で言うならば 「シンプルに考えられるコード」 であると、 私はそう考えています。
大きな仕様を、 干渉しあわない小さな仕様に分解できたのならば、 それだけシンプルに考えることが出来るようになっています。 それらを再び混ぜ合わせることは、 (そのメリットを上回る何か美味しい事が無い限りにおいて、)  リファクタリングの観点からは逆行していると言えます。

 

なお、 FizzBuzz の仕様が、
3の倍数のときに言う "Fizz" と
3の倍数かつ5の倍数のときに言う "Fizz Buzz" の "Fizz" は、 同じものであり、
5の倍数のときに言う "Buzz" と
3の倍数かつ5の倍数のときに言う "Fizz Buzz" の "Buzz" は、 同じものである
…ということならば、 私は "Fizz" と "Buzz" を定数にして、次のように書くでしょう。 (あ、 実際には、 メソッド切り出しはしないです。 )

private const string WordFor3の倍数 = "Fizz";
private const string WordFor5の倍数 = "Buzz";

private static string SayAt3と5の公倍数()
{
    return WordFor3の倍数 + " " + WordFor5の倍数;
}

private static string SayAt3の倍数()
{
    return WordFor3の倍数;
}

private static string SayAt5の倍数()
{
    return WordFor5の倍数;
}

これなら、 共通の定数に依存してはいますが、 他の仕様には依存していません。 SayAt3の倍数() や SayAt5の倍数() の仕様が変わったとしても、 SayAt3と5の公倍数() は影響を受けません。

|

« [コラム] リファクタリングはどっちへ進むべきだろう? (続々) | トップページ | [コラム] F# で NUnit するには »

*コラム」カテゴリの記事

コメント

コメントを書く



(ウェブ上には掲載しません)


コメントは記事投稿者が公開するまで表示されません。



トラックバック


この記事へのトラックバック一覧です: [コラム] リファクタリングはどっちへ進むべきだろう? (了):

« [コラム] リファクタリングはどっちへ進むべきだろう? (続々) | トップページ | [コラム] F# で NUnit するには »