HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Yahtzee program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
yahtzeeprogramstackoverflow

Problem

How can I refactor this?

```
require'yaml'
require 'text-table'

class Yahtzee
def initialize
puts "Welcome to Yahtzee, please enter your name."
@name = gets.chomp
@turn = 0
@scorecard = {"aces" => 0, "twos" => 0, "threes" => 0, "fours" => 0, "fives" => 0, "sixes" => 0, "3 of a kind" => 0, "4 of a kind" => 0, "full house" => 0, "sm. straight" => 0, "lg. straight" => 0, "chance" => 0, "yahtzee" => 0}
end

def roll
@roll_count = 1
@roll = Array.new(5) {rand(6) + 1}
p @roll
puts "That was your first roll, you are allowed 2 more rolls."
more_rolls?
end

def more_rolls?
puts "Would you like to roll again? (y or n)"
roll_again_choice = gets.chomp.downcase
if roll_again_choice == "y"
roll_again
elsif roll_again_choice == "n"
section(@roll)
else
more_rolls?
end
end
#Refactor for error on input of anything besides 1-5 and commas
def roll_again
puts "Which dice would you like to keep from this roll? (1, 2, 3, 4, 5)"
dice_to_keep = gets.chomp.split(',').map {|x| (x.to_i) - 1}.map {|x| @roll[x]}
new_roll = Array.new(5 - dice_to_keep.size) {rand(6) + 1}
@roll = new_roll + dice_to_keep
p @roll
@roll_count += 1
puts "That was roll number #{@roll_count}, you have #{3-@roll_count} remaining."
if @roll_count = 3}
@scorecard["3 of a kind"] = @roll.inject(:+)
else
puts "Your roll is not a 3 of a kind! Please select another section or type 'scratch' to score 0 for this section."
scratch = gets.chomp
if scratch == "scratch"
@scorecard["3 of a kind"] = "scratch"
elsif @scorecard.has_key?(scratch)
@turn -= 1
section_to_score(scratch)
else
three_of_a_kind
end
end
end

def four_of_a_kind
if @roll.map {|x| @roll.count(x)}.any? {|y| y >= 4}
@scorecard["4 of a kind"] = @roll.inject(:+)
else
puts "Your roll is not a 4 of a kind! Please select another

Solution

I would start with code formatting :D And replacing @scorecard initialization with following one (instead of explicit zerofication):

@scorecard = Hash.new{ |h,k| h[k] = 0 }


In nearly all cases you use gets, you puts before:

puts "Your roll is not a Full House!.."
scratch = gets.chomp


You can extract this into a dedicated method:

def ask message
  puts message
  gets.chomp
end


So that your calls will become:

scratch = ask "Your roll is not a Full House!.."


This place seems useless:

File.write('highscore.txt', hs.to_yaml)
hs_table = YAML.load_file('highscore.txt')


Final multiple maps can be replaced into something more beautiful, but we will need something to make ordinal numbers, we'll steel this method from ActiveSupport (Ruby on Rails):

def ordinalize(number)
  abs_number = number.to_i.abs

  if (11..13).include?(abs_number % 100)
    suffix = "th"
  else
    suffix = case abs_number % 10
      when 1; "st"
      when 2; "nd"
      when 3; "rd"
      else    "th"
    end
  end

  "#{number}#{suffix}"
end


Once we have this, we can refactor last maps into something like this:

hs_table = [["RANK", "PLAYER", "SCORE", "DATE"]]
(1..10).each do |rank|
  hs_table << [
    ordinalize(rank),
    hs_table[rank][:player],
    hs_table[rank][:score],
    hs_table[rank][:date]
  ]
end


Well at least these are first steps I would take. After first iteration, there might be more ideas more.

Code Snippets

@scorecard = Hash.new{ |h,k| h[k] = 0 }
puts "Your roll is not a Full House!.."
scratch = gets.chomp
def ask message
  puts message
  gets.chomp
end
scratch = ask "Your roll is not a Full House!.."
File.write('highscore.txt', hs.to_yaml)
hs_table = YAML.load_file('highscore.txt')

Context

StackExchange Code Review Q#28063, answer score: 4

Revisions (0)

No revisions yet.