Chapter 1

ようやく第一章。

ここではリファクタリングがどういうものか、ざっとデモのようなことをやって感じをつかむのが目的らしい。

「1.1 最初の状態」でビデオレンタルの料金印刷プログラムが提示される。この本はCD-ROMもついてないし、ネットからコードをダウンロードできるわけでもなさそうなので、大人しく「写経」する。

ところがいきなり誤植発見。

p.28の中央やや下、

  # 新作2日間レンタルでボーナス点を加算
  if element.movie.price_code == Movie.NEW_RELEASE && element.days_rented > 1
    frequent_renter_points += 1
  end

というコードがあるが、この"Movie.NEW_RELEASE"は"Movie::NEW_RELEASE"の誤りだと思う。

そもそも"Movie.NEW_RELEASE"だとrubyがエラーを起こすし。

「1.2 リファクタリングの第1歩」で、「リファクタリングする前にしっかりとしたテストスイートを用意しよう」とあるのだが、題材になっているビデオレンタルの料金印刷プログラムのテストスイートが全然載っていないのは、教則本としてちょっとどうかと思う。

仕方がないので自分でテストコードを書くのに挑戦した。TDDも初心者なので、ちょっと途方に暮れるところがあった。

この後しばらく話題の中心になるCustomerクラスのstatementメソッドをテストするコードを書くのに大いに難渋し、しまいにはメソッドの戻り値resultをインスタンス変数@resultに書き換えてからテストを書いてしまった。

一部変更してしまったCustomerクラスはこれ。

require 'Movie'
require 'Rental'

class Customer
  attr_reader :name
  attr_accessor :result
  
  def initialize(name)
    @name = name
    @rentals = []
  end
  
  def add_rental(arg)
    @rentals << arg
  end
  
  def statement
    total_amount, frequent_renter_points = 0, 0
    @result = "Rental Record for #{@name}¥n"
    @rentals.each do |element|
      this_amount = 0
      
      # 各行の金額を計算
      case element.movie.price_code
      when Movie::REGULAR
        this_amount += 2
        this_amount += (element.days_rented - 2) * 1.5 if element.days_rented > 2
      when Movie::NEW_RELEASE
        this_amount += element.days_rented * 3
      when Movie::CHILDRENS
        this_amount += 1.5
        this_amount += (element.days_rented - 3) * 1.5 if element.days_rented > 3
      end
      
      # レンタルポイントを加算
      frequent_renter_points += 1
      # 新作2日間でボーナス点を加算
      if element.movie.price_code == Movie::NEW_RELEASE && element.days_rented > 1
        frequent_renter_points += 1
      end
      # このレンタル料金を表示
      @result += "¥t" + element.movie.title + "¥t" + this_amount.to_s + "¥n"
      total_amount += this_amount
    end
    # フッター行を追加
    @result += "Amount owed is #{total_amount}¥n"
    @result += "You earned #{frequent_renter_points} frequent renter points"
    print @result
  end
end

後になって考えてみたら、@resultをインスタンス変数にしなくてもテストを書くのは可能だった……。
だがここは未熟なところを記録しておくためにインスタンス変数版で行くことにしよう。

Customer.rbをテストするTC_Customer.rbのコードは下記。

mustなるものがrequireされているが、これはRubyベストプラクティスの第一章に載っていたもの。

require 'rubygems'
require 'test/unit'
require 'must'
require 'Customer'
require 'kconv'

$KCODE="UTF8"
class TC_Customer < Test::Unit::TestCase

  def assert(status,msg)
    if(RUBY_PLATFORM.downcase =~ /mswin(?!ce)|mingw|cygwin|bccwin/)
      msg=Kconv.tosjis(msg)
    end
    super(status,msg)
  end

  def setup
    @obj = Customer.new("kono")
    @movie1 = Movie.new("Star Wars",Movie::REGULAR)
    @movie2 = Movie.new("District 9", Movie::NEW_RELEASE)
    @movie3 = Movie.new("Ponyo",Movie::CHILDRENS)
  end
  
  must "test1" do
    @obj.add_rental(Rental.new(@movie1, 7))
    @obj.add_rental(Rental.new(@movie2, 2))
    @obj.add_rental(Rental.new(@movie3, 5))
    @obj.statement
    result= "Rental Record for kono¥n¥tStar Wars¥t9.5¥n¥tDistrict 9¥t6¥n¥tPonyo¥t4.5¥nAmount owed is 20.0¥nYou earned 4 frequent renter points"
    assert_equal(result, @obj.result)
  end
end

テストは本当はもうちょっとしっかり書いた方が良いのだろうけど、この辺であまり時間をかけるわけにも行かないので「後でまた不都合が出たら考える」ことにして前に進む。

「1.3 statementメソッドの分解、再配置」
p.33-p.34のコードとp.35のコード。特に難しいところはなく、「写経」してテストをして動きを確認するのみ。

「1.4 金額計算ルーチンの移動」
p.37のコード、p.39のコード、p.41のコード。コードに難しいところはないが、p.41の解説に「等冪(とうべき)」という用語が出てきて、これは知らなかった。Wikipediaでは冪等性（べきとうせい、英: idempotence）という解説を見つけた。

「1.5 レンタルポイント計算メソッドの抽出」
p.43のコード。
p.44-p.45のシーケンス図を見て、「変更前」と「変更後」の違いは「図の違い」としては判るのだけど、図の意味するところがなかなかピンと来ないのは頭がオブジェクト指向になっていなからだろうか…。

「1.6 一時変数の削除」
p.47のコード、p.49のコード。
p.49のコードで使われているinjectメソッドは、慣れないと今ひとつ直感的ではないもののコードの記述量がぐっと減るのは確かに魅力である。
p.51のコード。
p.53の「まずコードをわかりやすくしてからプロファイラを使ってパフォーマンス問題に取り組む」べしという忠告は守るようにしたい。
p.55のコード。新たに追加したhtml_statementは、resultをクラス変数に変更することなくテストを書けた。メソッドの最後に

result

と一行だけ書くというやり方は一般的なのだろうか….?

「1.7 料金コードによる条件分岐からポリモーフィズムへ」
p.57のコード、p.59のコード。
p.56の冒頭、

他のオブジェクトの属性に基づいてcase文を書くのは間違っている。case文を使わなければならない場合には、他のオブジェクトではなく自分自身のデータに基づいてそうすべきだ。

という部分を読んだ瞬間「そんなこと言われなくても判ってる」と思ったのだが、これまで「写経」してきたソースがその「間違っている状態」であることに気づけなかったあたりがなんとも悔しい。

「1.8 ついに継承の導入か」
前節を読んだ後、二週間ほど経過してしまった。この節から内容的にも高度になる。UMLのシーケンス図を使った解説が登場し、GoFのデザインパターンの話題も出てくる。

ぼくはデザインパターンの本も買うだけは買ったがきちんと読んではいない。慌てて本棚から引っ張り出す。

結城さんのこの本は、分厚くてそう簡単に読めない。今回は「リファクタリング」の副読本的に読むことにする。UMLも日常的には使っていないのでシーケンス図も直感的には読解できないのだけど、並行してそちらももう少し慣れる必要がありそう。

p.63のコード。

def price_code=(value)
  @price_code = value
end

という記述が理解できなくて非常に戸惑う。最初に一読したとき、「これはvalueを引数にするprice_codeの特殊なメソッドなのだろうか。この=記号はなんだろう? こんな記述方法知らないなぁ…」等と考えて、メソッドの特別な記述方法を調べようとしたのだけど、いくら検索してもそんなものは出てこない。本の記述には

そこで、カスタムセッターメソッドを導入し(ここで面白いことをしようと思っているので)、コンストラクタからはそれを呼び出すことにする。

とあるが、その「カスタムセッターメソッド」の意味が判らなくて、難儀してしまった。

結局これは、Movieクラスのメンバ@price_codeに代入する演算子=をオーバーロードする指定なのだけど、p.63の

def price_code=(value)
  @price_code = value
end

という記述ではオーバーロード前と後で機能的な変化が何もないので却ってわかりづらくなってしまっている。p.67の

def price_code=(value)
 @price_code=value
 @price=case price_code
     when REGULAR: RegularPrice.new
     when NEW_RELEASE:NewReleasePrice.new
     when CHILDRENS: ChildrensPrice.new
  end
end

という記述になって、「"="演算子一発でprice_codeにvalueを代入すると同時にprice_codeに対応するクラスを@priceに代入する」という機能を持つようになり、オーバーロードする意味が出てくるのだが。

この辺、コードのtypo等のミスもあったのだけどユニットテストでは検出できず、後の方になってミスが発覚するというようなこともあった。テストコードの不備といえばそれまでなのだけど、RegularPrice/NewReleasePrice/ChildrensPriceは当初は何もしない空のクラスとして追加されるので、ユニットテストで検証することがなかなか難しい。ここはカバレッジツールの出番だろうと思って、rcovを導入しようとしたのだけど、Windows環境だとこれがなかなかうまく行かなくて、結局ubuntuで作業することにした。

p.67のコード。先述した理由で、このコードのテストを通すのがたぶん本章で一番大変だった。

p.69のコード。

p.71のコード。ここではinclude文が使われているが、これまでinclude文は使ったことがなかったので少しはまる。付け焼刃で参考にしたページ。

この本は、コードもポイントとなる部分しか書いてなくて、リファクタリングの過程で変更した部分であってもポイントでなければ記載が省略してあったりするようだ。p.73のコードもそうで、書籍に記載してある部分の他にMovieクラスのコンストラクタも変更する必要があった。

ともあれ、何とか第一章のおしまいまで到達。