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

2010年10月22日 (金)

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

さらに脱線します。

TDD しない前提で、 つまり、 しっかり考えてから一気にコードを書くとして、 FizzBuzz を見てみます。 まぁ、 メソッドの内部設計を、 TDD の力を借りずにやってみよう、 というわけ。

FizzBuzz の仕様はこうでした。

  • 3の倍数なら、 "Fizz" と言う。
  • 5の倍数なら、 "Buzz" と言う。
  • ただし、3の倍数 かつ 5の倍数なら、 "FizzBuzz" と言う。
  • それら以外は、 その数を言う。

これをよく考えてみると、 3番目の仕様の 「ただし、」 という文によって、 1番目と 2番目の仕様には、 書かれていない制限がなされています。 たとえば 2つめの仕様は、 5の倍数だったら、 いつでもまず "Buzz" と言っちゃっていいのか…? 違いますね。 5の倍数であっても、 同時に 3の倍数でもあったなら、 "Buzz" ではなく "FizzBuzz" と言わねばなりません。

前の 2つの仕様に書かれていない文を補えば、 「ただし」 を削ることができます。


  • 3の倍数であり かつ 5の倍数でない なら、 "Fizz" と言う。
  • 5の倍数であり かつ 3の倍数でない なら、 "Buzz" と言う。
  • 3の倍数であり かつ 5の倍数である なら、 "FizzBuzz" と言う。
  • それら以外は、 その数を言う。

1番目から 3番目までの仕様は、 直交しました 干渉しあわない独立した仕様になりました*1。 おたがいに影響を及ぼしあってはいないですよね?
※ こうなれば、 1番目から 3番目までの仕様を、 好きな順序でそれぞれ個別に実装しても OK です。 テストファーストで作りやすい仕様だとも言えます。
(*1) こういうときに 「直交」 という言葉はあまり使わないようなので、 修正。 (2010/10/23)

ところで、 FizzBuzz の仕様は、 正の整数すべてにあてはまる (まぁ、 Integer とかの上限値に縛られますけど) のですよね。 言い換えれば、 すべての正の整数を、 4つの集合に分類することになるでしょう。
その分類の仕方は?
3の倍数であるかないか、 5の倍数であるかないか、 を組み合わせればよさそうです。

 5の倍数である5の倍数でない
3の倍数である   
3の倍数でない   

ここに仕様の 1 ~ 3 が当てはまりますか?

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

これで合ってるかな?
「だから」 を消した仕様の 1 ~ 3 と、 しっかり見比べてみてください。 同じことが表現できているでしょうか?

すると、 表の空いている所は?
ここは、 「それら以外」 ですよね。 正の整数すべてを、 この表の 4マスに詰め込もうというのですから、 残ったところには、 それら以外全部が入ります。
何を入れればよいでしょう?
4番目の仕様 「それら以外は、 その数を言う」 ですね。

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

この表はデシジョン テーブル (decision table) になっています。 そして、 FizzBuzz の仕様を (正の整数の範囲で) 全て網羅できています。 正のすべての整数に対して、 すべての処理が定義できています。

仕様を厳密に表現できた (と、 思われる) ので、 あとはコードに直していくだけです。 まず、 擬似コードで表現してみます。

FizzBuzz:
    If 3の倍数である
        If 5の倍数である
            "FizzBuzz" と言う
        Else (5の倍数でない)
            "Fizz" と言う
    Else (3の倍数でない)
        If 5の倍数である
            "Buzz" と言う
        Else (5の倍数でない)
            その数を言う

この擬似コードで、 上の表を正確に表現できていますか?

それでは、 あとはこの擬似コードを C# に翻訳するだけですね。
そのときに、 "言う" というのは、 言うべきセリフを return することにしてしまいましょう。 また、 対象とする整数を引数 n で受け取ることにします。
すると、 こんなコードになるでしょう。
※ 0 または負のとき例外、 という仕様も追加してあります。

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

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

    if (is3の倍数)
    {
        if (is5の倍数)
        {
            return "FizzBuzz";
        }
        else
        {
            return "Fizz";
        }
    }
    else
    {
        if (is5の倍数)
        {
            return "Buzz";
        }
        else
        {
            return n.ToString();
        }
    }
}

上の擬似コードとの対比は説明を省きますが、 問題無いかと思います。

まぁ昔ながらのコードという感じで、 面白くはないですね。 なんていうか、 クソ真面目なコード?
デジジョン テーブルをそのまま表現しているので、 正確に仕様を表現しているハズなんですが、 いまいち分かりやすいとも思えません。 上に戻ってもらって、 デシジョン テーブルの元となった、「だから」 を消した仕様の文章を見てください。 厳密に表現できてはいますが、 分かりやすいとは言えない文章だとは思いませんか? コードも同じような感じがしますね。
# ここでユニット テスト コードが用意できていれば、 リファクタリングして If の入れ子や If ~ Else を減らして、 読みやすくできます。 昔は自動化テストなんてやってませんでしたから、 上のコードで完成、 です。

今回は、 TDD せずに、 昔ながらのデシジョン テーブルを作ってからコードに落とす、 というやり方で FizzBuzz を作ってみました。 まぁ、 こんな世界もある、 というか、 もはやロストワールドかもしれません。 でも私は、 ユニットテストを考えるときに、 デシジョン テーブルや入出力表をいまだに頭の中で描いています。

 

さらに余談。
この堅苦っしいコードの MSIL 見てみたら…

.method public hidebysig static string
        SayByDecidionTable(int32 n) cil managed
{
  // Code size       59 (0x3b)
  .maxstack  2
  .locals init ([0] bool 'is3の倍数', [1] bool 'is5の倍数')
  IL_0000:  ldarg.0
  IL_0001:  ldc.i4.0
  IL_0002:  bgt.s      IL_000a

  IL_0004:  newobj     instance void [mscorlib]System.ArgumentOutOfRangeException::.ctor()
  IL_0009:  throw

  IL_000a:  ldarg.0
  IL_000b:  ldc.i4.3
  IL_000c:  rem
  IL_000d:  ldc.i4.0
  IL_000e:  ceq
  IL_0010:  stloc.0
  IL_0011:  ldarg.0
  IL_0012:  ldc.i4.5
  IL_0013:  rem
  IL_0014:  ldc.i4.0
  IL_0015:  ceq
  IL_0017:  stloc.1
  IL_0018:  ldloc.0
  IL_0019:  brfalse.s  IL_002a

  IL_001b:  ldloc.1
  IL_001c:  brfalse.s  IL_0024

  IL_001e:  ldstr      "FizzBuzz"
  IL_0023:  ret

  IL_0024:  ldstr      "Fizz"
  IL_0029:  ret

  IL_002a:  ldloc.1
  IL_002b:  brfalse.s  IL_0033

  IL_002d:  ldstr      "Buzz"
  IL_0032:  ret

  IL_0033:  ldarga.s   n
  IL_0035:  call       instance string [mscorlib]System.Int32::ToString()
  IL_003a:  ret
} // end of method FizzBuzzer::SayByDecidionTable

なんと、 Code size = 59 (@@;
ここまでの最短でした! いやびっくり f(^^;

|

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

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

コメント

コメントを書く



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


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



トラックバック


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

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