7
\$\begingroup\$

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
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Sep 28, 2015 at 19:51
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

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.

answered Oct 1, 2015 at 1:33
\$\endgroup\$
1
  • \$\begingroup\$ Thank you @Borsuncho for your help. I especially agree with you about the guess_loop_comp and guess_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\$ Commented Oct 2, 2015 at 23:12
3
\$\begingroup\$

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 the Game, and by Knuth 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 change Board.

  • 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
answered Oct 1, 2015 at 19:35
\$\endgroup\$
1
  • \$\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 the Board class is also helpful. I'm new to OOP so it's good to get some solid direction. Thanks again! \$\endgroup\$ Commented Oct 2, 2015 at 23:07

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.