I've created a small command-line 'Mastermind' game using Ruby. The intent of the project (aside from having fun in the build!) was to learn and emphasize OOP concepts and principles.
The game has some simple AI in place. If the computer is the player making guesses, when they get the correct color in the correct index position, that guess is locked in for subsequent turns. When they get the correct color but in an incorrect index position, that color is utilized in the next turn but placed in a different (and unoccupied) index position.
module Mastermind
class Player
attr_accessor :color_choices
def initialize(color_choices)
@color_choices = color_choices
end
end
class Game
attr_accessor :computer, :human, :color_choices
def initialize
welcome
end
def play_game
@available = [0,1,2,3]
@computer = Player.new(get_random_choice)
human_instructions
choose_message
@human = Player.new(get_human_choice)
compare_with_index
@@guess_iterations = 1
@@guesses_hash = Hash.new
guess_loop_human
end
def comp_guesses
@available = [0,1,2,3]
choose_message
@human = Player.new(get_human_choice)
@computer = Player.new(get_random_choice)
compare_with_index
@@guess_iterations = 1
@@guesses_hash = Hash.new
guess_loop_comp
end
def welcome
puts "Welcome to Mastermind."
puts "========================================================"
who_creates_code
end
def human_instructions
puts "You will be given 12 chances to guess the code that was chosen by the computer."
puts "========================================================"
puts "There are 6 colors from which to choose (Red, Blue, Green, Yellow, Orange, Purple)"
puts "========================================================"
end
def who_creates_code
puts "Would you like to choose the code and have the computer guess? (yes/no)"
chooser = gets.chomp.downcase
input_validation(chooser)
end
def input_validation(response)
if response == "yes"
comp_guesses
elsif response == "no"
play_game
else
puts "Response not valid"
who_creates_code
end
end
def choose_message
puts "Please choose four colors, separated by a comma (ex: B,O,Y,G)"
end
def get_random_choice
puts "The computer is now choosing..."
colors = ["R", "B", "G", "Y", "O", "P"]
choice = colors.sample(4)
end
def get_human_choice
answer = gets.chomp.upcase
human_colors = answer.split(",")
end
def compare_with_index
@count_index = 0
@color_count = 0
computer.color_choices.each_with_index do |n, index|
if human.color_choices[index] == n
@count_index += 1
@available -= [index]
elsif color_match(n) && color_available(n)
@color_count += 1
end
end
@count_index
end
def color_match(color)
human.color_choices.include?(color)
end
def color_available(color)
@available.include?(human.color_choices.index(color))
end
def matches_message
"You have #{@count_index} color(s) in the right spot and #{@color_count} correctly chosen color(s)"
end
def guess_loop_comp
while @@guess_iterations <= 12 && !victory
store_guess_comp
board
puts matches_message
computer.color_choices = new_choice
compare_with_index
@@guess_iterations += 1
end
game_over
end
def guess_loop_human
while @@guess_iterations <= 12 && !victory
store_guess_human
board
puts matches_message
guess_again_message
human.color_choices = get_human_choice
compare_with_index
@@guess_iterations += 1
end
game_over
end
def store_guess_human
@@guesses_hash[human.color_choices] = matches_message
end
def store_guess_comp
@@guesses_hash[computer.color_choices] = matches_message
end
def board
Board.new
end
def guess_again_message
puts "Guess again, please choose four colors, separated by a comma (ex: B,O,Y,G)"
end
def available
@available.shuffle!.pop
end
def new_choice
colors = ["R", "B", "G", "Y", "O", "P"]
new_color = []
computer.color_choices.each_with_index do |n, index|
if human.color_choices[index] != n && !color_available(n)
new_color[index] = colors.sample
else
new_color[index] = n
end
end
new_color
keep_color(new_color)
end
def keep_color(new_array)
computer.color_choices.each do |i|
if color_available(i)
new_array[available.to_i] = i
end
end
p new_array
new_array
end
def victory
if compare_with_index == 4
store_guess_comp
board
puts "Victory!"
true
end
end
def game_over
if @@guess_iterations > 12 && !victory
puts "Game Over"
end
end
end
class Board < Game
def initialize
render_board
end
def render_board
(13- @@guess_iterations.to_i).times do
puts "| X | X | X | X |"
end
display_hash
end
def display_hash
@@guesses_hash.each do |k,v|
puts "================="
puts "| " + k.join(" | ") + " | " + v
puts "================="
end
end
end
end
Mastermind::Game.new
2 Answers 2
This is only a style issue, but it's first time I see Ruby code with empty lines at end of class body, and it seems confusing, as you are not separating other ends that way:
end
end
You have pairs of methods that are virtually identical, like #guess_loop_human
and #guess_loop_comp
. Repeating yourself is bad, this would be solved better if you had two classes HumanPlayer
and CPUPlayer
, moved player-specific logic there and in Game class referred to your players as 'player_a' and 'player_b', or 'setter' and 'guesser'.
module Player # could be a class
# common logic if any
end
class HumanPlayer # and CPUPlayer elsewhere
include Player
def get_choice
# possibly print messages here
end
end
def guess_loop # not guess_loop_human and guess_loop_comp
while @@guess_iterations <= 12 && !victory
store_guess
board
puts matches_message
guesser.get_choice # this works differently for HumanPlayer and CPUPlayer
compare_with_index
@@guess_iterations += 1
end
game_over
end
Your @guesses_hash
is a Hash indexed by Arrays. This is pretty bad, as two identical Arrays will count as the same key - that will mess your logic if player makes the same guess twice.
Your Board is subclass of Game, you used that to share class variables between them. This doesn't make sense though, as a board obviously isn't a game. What makes even less sense is that instantiating this class draws a board.
Both of above problems could be solved by introducing actual Board class that keeps track of guesses, makes this data available to Game, and can present it.
Structure of your methods is somewhat off sometimes. Lets look here:
def input_validation(response)
if response == "yes"
comp_guesses
elsif response == "no"
play_game
else
puts "Response not valid"
who_creates_code
end
end
Just because this method happens to be called in single place, it doesn't mean it should call methods that are supposed to be called after that - instead, such function should return a value, and it's caller should determine what to do with it. This makes this function reusable, just in case you would want to ask more yes\no question (like "Play again?(yes\no)"
, but also more readable - reader of your code wouldn't expect that input_validation
means input_validation_and_than_play_the_game
. For example you could do:
if get_yes_or_no # old name was confusing IMO
comp_guesses
else
play_game
end
#get_yes_or_no
above is assumed repeat #gets
in a loop until it gets valid answer, and I'd imagine it should also be the one calling chomp.upcase
.
-
\$\begingroup\$ Thank you @Borsuncho for your help. I especially agree with you about the
guess_loop_comp
andguess_loop_human
similarities. When I was building out the AI, I felt like I was in too deep to create and incorporate new classes so I tried this as a somewhat hacky workaround. Your separation is much more clear and DRY. Also, great catch about the@guesses_hash
. I hadn't even noticed that it wouldn't store repeat duplicate entries. Thanks again for your help. \$\endgroup\$scobo– scobo2015年10月02日 23:12:17 +00:00Commented Oct 2, 2015 at 23:12
I'd separate responsibilities further. Right now, Game
does almost everything including acting as both kinds of players, UI, and keeping game state.
Ideally, one class would focus on the game state (the Board
class would be a good fit), the different players can be separate classes that implement whatever logic they require. Players really only need to respond with a guess, and (depending on the player) know the state of play. Game
could be the overall controller in charge of feeding players' guesses to the board, and the feeding resulting hints back to the players.
Also, a definition like this smells:
class Board < Game
A board isn't a game. A board is just a board. There's no reason for it to inherit all the functionality of Game
. In fact, it doesn't work. If I do:
Mastermind::Board.new
I get NameError: uninitialized class variable @@guess_iterations in Mastermind::Board
.
In other words, you've split you code into two classes, but one simply cannot work with the other having done something first, despite the two classes ostensibly being separate things. And - not that this matters as much, but it's a strong code smell - you can't have two games running at the same time, since the class variables are shared between all instances.
So, in effect, you don't really have two classes. You just have a couple of board-related methods you've put somewhere else.
It's in the same vein as what @Borsunho pointed out: You have a lot of code that either assumes or enforces one specific sequence of calls, meaning you can't use (or test) parts in isolation. For instance, you can't create a new Game
without it automatically calling welcome
, which in turn calls who_creates_code
, which in turn... etc..
Also, (unrelated to the structure and OOP and all that) this approach isn't safe:
choice = colors.sample(4)
Calling sample
with a number will cause it to pick n unique elements from the array. I.e. the code can never be "R,R,G,O" because a color can't repeat. This cuts the possible codes from 1296 color combinations to just 360, if my math is correct.
I'll just try something here. This is less review, and more just me playing around. I've tried creating a more simplistic version that only does the AI player part. I've used a naïve version of Knuth's 5-guess algorithm because it's easy to implement, and might help illustrate the virtues of code reuse (if I do it right).
Here's the code:
class Board
COLORS = %w(R G B Y O P).freeze
CODE_LENGTH = 4
attr_writer :code
attr_reader :last_guess, :last_result
def initialize
randomize! # default to using a random code
end
def randomize!
@code = CODE_LENGTH.times.map { COLORS.sample }
end
def try(guess)
@last_guess = guess
# pair up code and guess, and divide into exact matches and everything else
hits, remaining = @code.zip(guess).partition { |a, b| a == b }
# get the unaccounted-for code parts and guess parts
# FIXME: I'm overwriting variables here, which isn't pretty
code, guess = remaining.transpose
# compare with the remaining guess parts (if any)
misses = (guess || []).count do |guess|
index = code.index(guess)
code.delete_at(index) if index
end
@last_result = [hits.count, misses]
end
end
class Knuth
def initialize
@possibilities = Board::COLORS.repeated_permutation(Board::CODE_LENGTH).to_a
@guess = nil
end
def guess(previous_result)
cull_possibilities(previous_result) if previous_result
index = rand([email protected])
@guess = @possibilities.delete_at(index)
end
def to_s
"Knuth"
end
private
# Knuth's algorithm: Compare your last guess against all the other possibilities.
# The secret code will be one of those that give the same result against your guess
# as your guess did against the actual secret code.
# Note that this uses the very same Board class.
def cull_possibilities(result)
board = Board.new
board.code = @guess
@possibilities.select! { |possible| board.try(possible) == result }
end
end
class Game
MAX_TRIES = 12
attr_reader :player, :board
def initialize(player)
@board = Board.new
@player = player # use object composition
end
def play
puts "The secret code is #{board.randomize!.join}. Don't tell anyone!"
last_result = nil
(1..MAX_TRIES).each do |number|
guess = player.guess(last_result)
last_result = board.try(guess)
puts "Guess #{number}: #{player} tried #{guess.join}, and got #{last_result.first} correct, #{last_result.last} close"
if last_result.first == Board::CODE_LENGTH
puts "#{player} wins in #{number} attempts!"
return
end
end
puts "#{player} failed to guess the code in #{MAX_TRIES} attempts..."
end
end
# Create a game, give it a player, and play
Game.new(Knuth.new).play
Obviously, I'm skipping a lot of stuff that you had (like, you know, being able to play it yourself). And there's a lot of this that doesn't smell quite right to me either (there's some suspicious coupling here and there and no one's keeping a full history of guesses), but the points I'm trying to illustrate are:
Generic building blocks. The
Board
class does very little (it just keeps a secret code and checks guesses), so it can be used by both theGame
, and byKnuth
albeit for different purposes. The board doesn't care what it's used for. Also, if you want to add a seventh color, or make the code longer, go for it; you only have to changeBoard
.Object composition and duck typing. The game just needs a player in order to play. It really doesn't care what object you give it, as long as that object at least responds to the message
guess
. So you can create another player class that takes user input, or uses a different algorithm, and pass that to the game instead. It just has to produce guesses.Use constants instead of magic/repeated numbers and values.
It usually guesses things in 5 attempts or less (as the name of the algorithm suggests), but because it's a naïve implementation that doesn't sort the possibilities, it sometimes goes over.
Again, this was as much for myself as anything (I just like Knuth's algorithm), but hopefully you'll at least find it interesting.
Just for fun, here are a couple of simple (i.e. really dumb) players you can swap in. They tend to lose.
# Always tries the same code. Always. One day, it'll win in 1 guess.
class StubbornPlayer
def initialize
@my_code = Board::COLORS[0, 4]
end
def guess(*args)
@my_code
end
def to_s
"The stubborn player"
end
end
# Just guesses randomly. It doesn't have much luck.
class Scatterbrain
def guess(*args)
Board.new.randomize!
end
def to_s
"Scatterbrain"
end
end
-
\$\begingroup\$ Thanks so much @Flambino for the exhaustive, detailed breakdowns and tips. Explaining the Knuth algorithm is especially helpful, I had no idea my
sample
was limiting the possibilities but it make so much sense now. The tip about narrowing the scope of theBoard
class is also helpful. I'm new to OOP so it's good to get some solid direction. Thanks again! \$\endgroup\$scobo– scobo2015年10月02日 23:07:38 +00:00Commented Oct 2, 2015 at 23:07