実践で学ぶオブジェクト指向⑤ テスト実践編(Rspec)


前回学んだことを踏まえてRspecで各モデルのテストを書きました。

Gameモデルのspecを最後に書いたんですが、1つのメソッドで副作用がたくさん起こるのでテストが書きづらかった。
「テストを書くのに苦労するとき、アプリケーションの設計がマズい可能性がある」とありましたが、これに当てはまるパターンでした。

1メソッド3行までというルールに基づいて書くと、とてもわかり易くなりました。


変更前のメソッド

def already_limit?
  if counter.limit?(opened)
    board.face_down(opened)
    player.face_down
  else
    face_up_selected?(card_id)
  end
  
  now_limit?(card_id) #追加
end

def face_up_selected?(card_id)
  i = card_id * 3
  if cards[i] == "-"
    board.face_up(i)
    player.face_up(i)
  end
end

def now_limit?(card_id)
  if counter.limit?(opened)
    judge_cards(index(card_id))
  end
end

def judge_cards(i)
  if Judge.new.judge(cards[opened[0]+2], cards[i+2])
    player.face_down
  end
end

ユーザーがカードをクリックしてから、カードがオープンされるまでの処理です。
間には

  • counter.limit?(opened)で既に2枚引いているかを確認し、そうであればその2枚を裏返して処理を抜ける

  • クリックしたカードが現在裏向きであれば、そのカードを表向きにする

  • 上記処理を終えた時点で再度表向きのカードが2枚になったかを確認し、そうであれば2枚の数字部分が一致するかを判定する

  • 一致したらplayerオブジェクトが持つopened情報を削除する

などの色々な処理を含みます。
さらに、already_limit?メソッドの最後でnow_limit?メソッドを呼び出す、というような実装になっているので再利用ができないという問題もありました。


変更後のメソッド

attr_reader :board, :player, :card_id

def initialize(args)
    @board = args[:board]
    @player = args[:player]
    @card_id = args[:card_id] 
    card_data?
  end
  
  def card_data?
    select if card_id
  end
  



  def select 
    return if face_down_2_cards 
    face_up_selected 
    judge_2_cards 
  end
  
  def face_down_2_cards
    face_down if counter.limit?(opened) 
  end
  
  def face_down 
    board.face_down(opened) 
    player.face_down 
  end

  def face_up_selected
    face_up if !counter.limit?(opened) 
  end
  
  def face_up 
    if judge.face_down?(cards[ index ] ) 
      board.face_up( index ) 
      player.face_up( index ) 
    end
  end

  def judge_2_cards
    flip_over if counter.limit?(opened) 
  end

  def flip_over 
    if judge.same_cards?(cards[opened[0]+2], cards[index+2]) 
      player.face_down 
    end
  end


selectメソッドが一連の流れを知識として持ち、それぞれのメソッドを呼び出すという形にしました。
「実践で学ぶオブジェクト指向③ メソッドの粒度」で出てきたピラミッド的な構造の案です。


1つのメソッドがシンプルになったので、これでテストは書きやすいと思います。
しかし、命名がイマイチなのか、そもそも別クラスに切り出すべき部分があるのか、あまりスマートな感じはしません。


あと、card_idが何度も出てきていたのでこれをGameクラスの知識として初期化時に生成するようにしました。
それに伴って、今までコントローラでplayerオブジェクトに対してcard_idデータを送り、そこからGameのalready_limit?を呼び出していたのをGameオブジェクト作成時にプライベートメソッドを呼ぶ形で起動させるようにしました。
これはクリックする毎にセッションの情報で初期化したGameオブジェクトを生成しているためです。

class GameController < ApplicationController
  def index
    @game = Game.new(
      :board => Board.new(nil), 
      :player => Player.new(nil)
      )

      session[:game] = @game.get
  end

  def show
    @game = Game.new(
      :board  => Board.new(session[:game]),
      :player => Player.new(session[:game]),
      :card_id => params[:id].to_i
      )
      
    session[:game] = @game.get
  
    render :index  
  end
end



プライベートメソッドのテストを書こうとして失敗


  def select(card_id) #パブリックインターフェース
    return if face_down_2_cards #送信コマンド
    face_up_selected(card_id) #送信コマンド
    judge_2_cards(card_id) #送信コマンド
  end

このselectメソッドのテストを書きたくて丸1日ウンウン唸ってました。


何をテストしたかったかというと、「selectにおいてface_down_2_cardsがfalseを返せばそこでreturnするかどうか」ということです。


奮闘した結果、それはrubyのreturn(或いは if)メソッドのテストだということに気付きました。

さらに、プライベートメソッドの送信テスト(呼び出しテスト)も同じようにrubyのselfメソッドのテスト であることにも気付きました……。


送信コマンドメッセージのテスト


ということで、Gameクラスのテストとして行うべきは、playerオブジェクトとboardオブジェクトに対するメッセージ送信のテストのみだと判断しました。

ちなみに本の中では「メッセージ送信のテスト」とありますが、一般的には「メソッド呼び出しのテスト」と言うようです。


例としてface_upメソッドのspecを挙げます。

  def face_up 
    if judge.face_down?(cards[ index ] ) 
      board.face_up( index ) #これをテストする
      player.face_up( index ) #これをテストする
    end
  end


  describe "#face_up" do
    #モックオブジェクトの用意
    let(:player_mock){ double('player') }
    let(:board_mock){ double('board') }
    let(:judge_mock){ double('judge') }
    
    it "submit message #face_up to board and player"do
      # if分岐をスルー  
      allow(judge_mock).to receive(:face_down?).and_return true
      
      #gameがそれぞれのオブジェクトを呼び出したら、モックを呼ぶように設定
      game =  Game.new(player: build(:player, :have_1_opened), board: build(:board), card_id: 1) 
      allow(game).to receive(:player).and_return(player_mock)
      allow(game).to receive(:board).and_return(board_mock)
      allow(game).to receive(:judge).and_return(judge_mock)
      
      #期待する動作を記述
      expect(player_mock).to receive(:face_up).with(3).once
      expect(board_mock).to receive(:face_up).with(3).once
  
      
      # Board#face_upでエラーになるのを回避
      allow(board_mock).to receive(:cards).and_return "+"
      
      #メソッドを実行
      game.face_up
    end
  end

メッセージ送信のテストで、モックを作成する所までは分かっていたんですが、
そのモックに、テスト対象のメソッドを受信するかどうか記述する(expect部分)だけじゃなくて、
gameオブジェクトに対しても、例えばboardが呼ばれたらモックを返すようにするんだぞ、と教える必要があるんですね。(allow(game)~部分)
これが分からずに結構時間を使いました。



受信メッセージのテスト


上記のspecではGameクラスからboardオブジェクトやplayerオブジェクトにメッセージが送信されることをテストしました。

ではそのboard.face_upがきちんと実行されることもテストしましょう。
もちろん、これはBoardモデルのspecとして書きます。


describe "#face_up" do
    let(:board){ build(:board) }
    it { expect{ board.face_up(6)}.to change{ board.cards[6]}.from("-").to("+") }
  end

こっちは簡単ですね。
build(:board)でFactoryBotで生成したboardオブジェクトを呼び出します。
そして、face_upメソッドによって、boardオブジェクトの持つcardsの6番目の文字が"-"から"+"になることを確認します。



終わりに

他の受信メッセージもこんな感じでテストを書いていきました。
オブジェクト指向設計を考慮したテストを書くのが初めてだったのと、Gameクラスのメソッドが大きすぎてテストを書くにあたって改修が必要だったためかなり苦労してテストを書きました。
本の中で例に出ているのがminiTestだったのも地味にキツかった。


本を読んで一応試してみて、最初の1つのクラスにすべての処理を書いてた頃よりは断然良くなったと思うけど、まだ臭う部分があります。
Gameクラスにインスタンス変数が3つあるのと、メソッドを細かく切り出したので80行のクラスになっていることです。

やはり一部分を別のクラスに切り出す必要があると思うのですが、この神経衰弱アプリケーションに関しては一旦ここで一段落つけることにします。
次は継承とダックタイプとコンポジションを活用するようなプログラムを書きながらそれぞれについて学ぼうかな。