patternrubyMinor
Yahtzee program
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
```
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
In nearly all cases you use
You can extract this into a dedicated method:
So that your calls will become:
This place seems useless:
Final multiple
Once we have this, we can refactor last maps into something like this:
Well at least these are first steps I would take. After first iteration, there might be more ideas more.
@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.chompYou can extract this into a dedicated method:
def ask message
puts message
gets.chomp
endSo 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}"
endOnce 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]
]
endWell 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.chompdef ask message
puts message
gets.chomp
endscratch = 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.