Solution to simple refactoring exercise.

refactoring_solution_mehowte.rb

# This is a simple refactoring exercise.
#
# What to do?
#
# 1. Look at the code of the class CorrectAnswerBehavior
# 2. Try to see what it does by running `ruby refactoring_example.rb` 
# 3. Record characterisation tests by running `ruby refactoring_example.rb --record`
# 4. Make the code beautiful;)
# 5. You are allowed to modify only the code between markers (REFACTORING START/REFACTORING END).
# 6. Test must pass! You can run them with command `ruby refactoring_example.rb --record`
# 7. For suggestions of other exercises based on this code...
#     a) Follow http://twitter.com/programmingwod or
#     b) like https://www.facebook.com/ProgrammingWorkout or
#     c) signup to http://programmingworkout.com
#
# Usage:
#  ruby refactoring_example.rb [-h|--help|help]       - shows help screen.
#  ruby refactoring_example.rb [-c|--clean|clean]     - clean recorded results of simulation.
#  ruby refactoring_example.rb [-r|--record|record]   - records 5000 results of simulation.
#  ruby refactoring_example.rb [-t|--test|test]       - tests against 5000 recorded results of simulation.
#  ruby refactoring_example.rb <ANY_NUMBER>           - shows result of simulation initialized with <ANY_NUMBER>.
#  ruby refactoring_example.rb                        - shows result of random simulation.
# 
# License: MIT (see at the end of the file)
# This code is based on Trivia Game example used in Legacy Code Retreats
# You can find it at https://github.com/jbrains/trivia

# ------------------------------ REFACTORING START ------------------------------
class CorrectAnswerBehavior

  def was_correctly_answered
    notify_about_penalty_box_situation
    notify_about_correct_answer
    reward_current_player
    result = current_player_won_or_is_penalized?
    change_player
    notify_about_player_change
    result
  end

  private

   def reward_current_player
    unless current_player_stays_in_penalty_box?
      increment_current_player_coins_count
      notify_about_current_player_coins_count
    end
  end
 

  def notify_about_correct_answer
    # TODO: Inquire about possible incorrect behaviour (typo in word "correct").
    if is_current_player_in_penalty_box?
      if is_current_player_getting_out_of_penalty_box?
        puts 'Answer was correct!!!!'
      end
    else
      puts "Answer was corrent!!!!"
    end
  end

  def notify_about_penalty_box_situation 
    if is_current_player_in_penalty_box?
      if is_current_player_getting_out_of_penalty_box?
        puts "#{current_player_name} got out of penalty box"
      else
        puts "#{current_player_name} stays in penalty box"
      end
    end
  end

  def notify_about_current_player_coins_count
    puts "#{current_player_name} now has #{current_player_coins_count} Gold Coins."
  end

  def notify_about_player_change
    puts "Player is now #{current_player_name}"
  end


 
  def current_player_won_or_is_penalized?
    current_player_stays_in_penalty_box? or did_player_win?
  end

  def current_player_stays_in_penalty_box?
    is_current_player_in_penalty_box? and not is_current_player_getting_out_of_penalty_box?
  end

  def did_player_win?
    current_player_coins_count != 6
  end

  def is_current_player_getting_out_of_penalty_box?
    @is_getting_out_of_penalty_box
  end

  def is_current_player_in_penalty_box?
    @in_penalty_box[@current_player]
  end


  def current_player_name
    @players[@current_player]
  end

  def change_player
    @current_player = (@current_player + 1) % @players.length
  end

  def increment_current_player_coins_count
    @purses[@current_player] += 1
  end

  def current_player_coins_count
    @purses[@current_player]
  end
# ------------------------------ REFACTORING END ------------------------------

public
  def initialize seed = nil
    srand(seed) if seed
    @players = %w[Alice Bob Cecil]
    @purses = @players.map { rand(3) + 2 }
    @in_penalty_box = @players.map { rand(2) == 0 }
    @current_player = rand(@players.count)
    @is_getting_out_of_penalty_box = @in_penalty_box[@current_player] && rand(2) == 0
  end
end

require 'fileutils'
module FixtureHandler
  def self.clear_fixtures
    FileUtils.rm_rf(fixtures_dir)
  end

  def self.create_fixture_dir
    FileUtils.mkdir(fixtures_dir)
  end

  def self.write_fixture index, text
    File.open(fixture_path(index), "w") do |file|
      file.write(text)
    end
  end

  def self.fixture_exists? index
    File.exists?(fixture_path(index))
  end

  def self.read_fixture index
    File.read(fixture_path(index))
  end

  def self.fixture_path index
    "#{fixtures_dir}/#{index}.txt"
  end

  def self.fixtures_dir
    "#{File.expand_path(File.dirname(__FILE__))}/fixtures"
  end
end

module StdOutToStringRedirector
  require 'stringio'
  def self.redirect_stdout_to_string
    sio = StringIO.new
    old_stdout, $stdout = $stdout, sio
    yield  
    $stdout = old_stdout 
    sio.string
  end
end

SIMULATIONS_COUNT = 5000
def run_simulation index = nil
  CorrectAnswerBehavior.new(index).was_correctly_answered
end

def capture_simulation_output index
  StdOutToStringRedirector.redirect_stdout_to_string do
    run_simulation(index)
  end
end

def clean_fixtures
  FixtureHandler.clear_fixtures
end

def record_fixtures
  SIMULATIONS_COUNT.times do |index|
    raise "You need to clean recorded simulation results first!" if FixtureHandler.fixture_exists?(index)
  end
  FixtureHandler.create_fixture_dir
  SIMULATIONS_COUNT.times do |index|
    FixtureHandler.write_fixture(index, capture_simulation_output(index))
  end
rescue RuntimeError => e
  puts "ERROR!!!"
  puts e.message
end

require 'test/unit/assertions'
include Test::Unit::Assertions
def test_output
  SIMULATIONS_COUNT.times do |index|
    raise "You need to record simulation results first!" unless FixtureHandler.fixture_exists?(index)
    assert_equal(FixtureHandler.read_fixture(index), capture_simulation_output(index))
  end
  puts "OK."
rescue RuntimeError => e
  puts "ERROR!!!"
  puts e.message
end

case ARGV[0].to_s.downcase
when "-h", "--help", "help"
  puts "Usage:"
  puts "  ruby #{__FILE__} [-h|--help|help]       - shows help screen."
  puts "  ruby #{__FILE__} [-c|--clean|clean]     - clean recorded results of simulation."
  puts "  ruby #{__FILE__} [-r|--record|record]   - records #{SIMULATIONS_COUNT} results of simulation."
  puts "  ruby #{__FILE__} [-t|--test|test]       - tests against #{SIMULATIONS_COUNT} recorded results of simulation."
  puts "  ruby #{__FILE__} <ANY_NUMBER>           - shows result of simulation initialized with <ANY_NUMBER>."
  puts "  ruby #{__FILE__}                        - shows result of random simulation."
when "-c", "--clean", "clean"
  clean_fixtures
when "-r", "--record", "record"
  record_fixtures
when "-t", "--test", "test"
  test_output
when /\d(.\d+)?/
  run_simulation ARGV[0].to_f
else 
  run_simulation
end


# Copyright © 2012 Michal Taszycki
#
# Permission is hereby granted, free of charge, to any person obtaining
# a copy of this software and associated documentation files (the
# "Software"), to deal in the Software without restriction, including
# without limitation the rights to use, copy, modify, merge, publish,
# distribute, sublicense, and/or sell copies of the Software, and to
# permit persons to whom the Software is furnished to do so, subject to
# the following conditions:
#
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

cool! I have some comments, though:

• I like the 'imperative' mode, of telling what to do in was_correctly_answered but all the ìfs inside the methods confuse me. Wouldn't it be better to have a main ìf/else and simplify the 'main' loop?
• I don't like much the _or_ (and _and_s) in method names, as in current_player_won_or_is_penalized?. Maybe something more general, such as current_player_status...
• I didn't have the courage to do so, but... what about adding some objects to this system, such as player, purse, penalty_box and a simple notification system?

Regards!

• The main thing that bothers me with one large if in was_correctly_answered is that it combines multiple responsibilities together.
  I.e. notifications with changing the current player and rewards. I personally prefer to split the responsibilities for the price of having multiple similarly looking if statements in multiple methods. One advantage of this is that I could now easily extract responsibilities into other classes. E.g. some kind of notifier, players collection, purse etc.
• Good point! I don't like them as well. In this case we have too little information on what does player_win even mean so I've decided to stick with the naive name. I like your suggestion though current_player_status feels better than current one.
• I was wondering if anyone will be adventurous enough with adding other classes or modules. That's why I left "REFACTORING START" marker before the class definition. The problem is that the initialization function is untouchable and it directly sets instance variables so any object initialization would have to happen at the beginning of the was_correctly_answered and still copy the information from instance variables. It does not seem rational.

Maybe after allowing changes to initialization function itself we could see some nice object extractions. Sounds like an idea for exercise III. :D