レガシーコードのリファクタリングの痛みを減らすパラメタライズドテストの活用について

この世界には二種類のソフトウェア開発者しかいない。
テストを書いてからリファクタするソフトウェア開発者と、テストを書かずにリファクタするソフトウェア開発者だ。

さらに、テストを書いてからリファクタするソフトウェア開発者には二種類のソフトウェア開発者しかいない。
テストをパラメタライズするソフトウェア開発者と、しないソフトウェア開発者だ。

※煽りとかじゃなくてネタです。テスト時にパラメタライズドテストを推奨する意図があるわけではないです。

ということでレガシーコードに対してパラメタライズドテストを書いてからリファクタします。

前提

レガシーコードとは？

ここではテストコードがないコードのことを指すものとします。
ちなみに、私は書籍「レガシーコード改善ガイド」を読んだことがありません。

レガシーコードをリファクタリングする

レガシーコードをリファクタする場合、一般的にテストコードで保護することになります。
ここで何故？という方は以下のスライドがわかりやすいです。

engineer.crowdworks.jp

手順

パラメタライズドテストを使ってレガシーコードをリファクタリングする手順をまとめます。

テスト対象コード

FizzBuzz をテスト対象とします。言語は Ruby で書いてあります。
FizzBuzz についてはこちらなどをご確認ください。

d.hatena.ne.jp

以下のコードに関して突っ込みどころはあると思いますが、
この記事の内容としてはテスト内容自体はさほど重要ではないのでスルー推奨です。

class FizzBuzz
  def fizzbuzz(limit)
    (1..limit).each_with_object([]) do |i, memo|
      memo << case
      when i % 15 == 0 then "FizzBuzz"
      when i % 5 == 0 then "Buzz"
      when i % 3 == 0 then "Fizz"
      else i.to_s
      end
    end
  end
end

テストを作成する

RSpecでテストを作成します。パラメタライズドテストは話をシンプルにするために配列を使います。
実際にRSpecでパラメタライズドテストをする場合は rspec-parameterized を使う方が多いと思います。
ここでは期待値を ほぼ確実に出力されない値 にしておきます。

require 'spec_helper'
require 'fizzbuzz'

describe FizzBuzz do
  describe '#fizzbuzz' do
    cases = [
      { case: "Fizz", limit: 3, expected: [:TODO] },
      { case: "Fizz and Buzz", limit: 5, expected: [:TODO] },
      { case: "Fizz , Buzz and FizzBuzz", limit: 15, expected: [:TODO] }
    ]

    cases.each do |c|
      it c[:case] do
        actual = FizzBuzz.new.fizzbuzz(c[:limit])
        expect(actual).to eq(c[:expected])
      end
    end
  end
end

テストを実行します

ほぼ確実に出力されない値 でテストしたため、テスト結果はすべて fail します。
これにより、RSpecが期待値と実際の出力を表示してくれます。

rspec
FFF

Failures:

  1) FizzBuzz#fizzbuzz Fizz
     Failure/Error: expect(actual).to eq(c[:expected])

       expected: []
            got: ["1", "2", "Fizz"]

       (compared using ==)
     # ./spec/fizzbuzz_spec.rb:18:in `block (4 levels) in <top (required)>'

  2) FizzBuzz#fizzbuzz Fizz and Buzz
     Failure/Error: expect(actual).to eq(c[:expected])

       expected: []
            got: ["1", "2", "Fizz", "4", "Buzz"]

       (compared using ==)
     # ./spec/fizzbuzz_spec.rb:18:in `block (4 levels) in <top (required)>'

  3) FizzBuzz#fizzbuzz Fizz , Buzz and FizzBuzz
     Failure/Error: expect(actual).to eq(c[:expected])

       expected: []
            got: ["1", "2", "Fizz", "4", "Buzz", "Fizz", "7", "8", "Fizz", "Buzz", "11", "Fizz", "13", "14", "FizzBuzz"]

       (compared using ==)
     # ./spec/fizzbuzz_spec.rb:18:in `block (4 levels) in <top (required)>'

Finished in 0.0015 seconds (files took 0.25571 seconds to load)
3 examples, 3 failures

Failed examples:

rspec ./spec/fizzbuzz_spec.rb:16 # FizzBuzz#fizzbuzz Fizz
rspec ./spec/fizzbuzz_spec.rb:16 # FizzBuzz#fizzbuzz Fizz and Buzz
rspec ./spec/fizzbuzz_spec.rb:16 # FizzBuzz#fizzbuzz Fizz , Buzz and FizzBuzz

テスト結果を元に期待値を設定する

先程行ったテスト結果に表示された実行結果を期待値に設定してテストを再実行します。

• 修正版のテスト

require 'spec_helper'
require 'fizzbuzz'

describe FizzBuzz do
  describe '#fizzbuzz' do
    cases = [
      { case: "Fizz", limit: 3, expected: ["1", "2", "Fizz"] },
      { case: "Fizz and Buzz", limit: 5, expected: ["1", "2", "Fizz", "4", "Buzz"] },
      { case: "Fizz , Buzz and FizzBuzz", limit: 15, expected: ["1", "2", "Fizz", "4", "Buzz", "Fizz", "7", "8", "Fizz", "Buzz", "11", "Fizz", "13", "14", "FizzBuzz"] }
    ]

    cases.each do |c|
      it c[:case] do
        actual = FizzBuzz.new.fizzbuzz(c[:limit])
        expect(actual).to eq(c[:expected])
      end
    end
  end
end

• 修正版のテストを実行する

$ rspec
...

Finished in 0.0015 seconds (files took 0.23866 seconds to load)
3 examples, 0 failures

リファクタする

テストで保護できたので、気兼ねなくリファクタします。
FizzBuzzの判定を lambda にしました。特に変更前より良くする、という意図はなくて
単に何かしらの変更をしましたよ、ということなので詳細は気にしないでください。

class FizzBuzz
  def fizzbuzz(limit)
    (1..limit).each_with_object([]) do |i, memo|
      memo << case i
      when fizzbuzz? then "FizzBuzz"
      when buzz? then "Buzz"
      when fizz? then "Fizz"
      else i.to_s
      end
    end
  end

  private

  def fizzbuzz?
    ->(i){ i % 15 == 0 }
  end

  def buzz?
    ->(i){ i % 5 == 0 }
  end

  def fizz?
    ->(i){ i % 3 == 0 }
  end
end

テストする

あらかじめ作成したテストを実行します。
無事、出力を変えずに変更することができました。

$ rspec
...

Finished in 0.0015 seconds (files took 0.24216 seconds to load)
3 examples, 0 failures

まとめ

レガシーコードの変更はゲーム終了間際のジェンガのような緊張感がつきまといます。
テストで既存の動きを保証することで、勇気をもってコードベースを改善することができます。

そしてレガシーコードは一般的に複雑で好ましくない設計をしていることが多いです。
つまり、1関数（メソッド）におけるテストケースが非常に多いことがよくあります。
その際に既存テストをテストコードとして1ケース1ケースベタ書きするのは手間がかかります。
この「面倒さ」が障壁になってリファクタを諦めるケースすらあるかもしれません。

そこで、パラメタライズドテストにすることによって、
ひとつのテストケースを作成して、あとはひたすらテストケースのパラメーターを埋めるだけになります。
（パラメタライズドテストがしにくい箇所があると思うので、可能な範囲でという話）

また、コードベース改善の流れ上、レガシーコードのリファクタによってメソッドやクラスを抽出・分割するケースが多いです。
この際にもテストケースをパラメタライズしておくと任意のテストケースをコピペで移動して、
それを少し加工して使いまわす際の手間が小さくなります。
ケースをべた書きしていた場合はそのあたりの手間がより大きいように思います。

以上のようなケースを想定して、レガシーコードのリファクタリングテクニックの一つとしての
パラメタライズドテストのご紹介でした。