Ruby command line Mastermind game with AI

Question

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

Answer 1

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.

Answer 2

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