Ruby Command Line Tic-Tac-Toe The following is my implementation of a command-line Tic-Tac-Toe game, written in Ruby. This was my first attempt at practicing object-oriented design principles.

require 'colored'

module TicTacToe
  class Player 
    attr_accessor :symbol
    def initialize(symbol)
      @symbol = symbol
    end
  end

  class Board 
    attr_reader :spaces
    def initialize
      @spaces = Array.new(9)
    end

    def to_s
      output = ""
      0.upto(8) do |position|
        output &lt;&lt; "#{spaces[position] || position}"
        case position % 3 
        when 0, 1 then output &lt;&lt; " | "
        when 2 then output &lt;&lt; "\n-----------\n" unless position == 8
        end
      end
      output
    end

    def check_space(cell, sym)
      if spaces[cell].nil?
        place_symbol(cell, sym)
        @current_turn += 1
      else
        puts "Space unavailable! Please select another cell"
      end
    end

    def place_symbol(cell, sym)
      spaces[cell] = sym
    end

    WINNING_COMBOS = [
      [0, 1, 2], [3, 4, 5], [6, 7, 8],
      [0, 3, 6], [1, 4, 7], [2, 5, 8],
      [0, 4, 8], [2, 4, 6]
    ]

    def winning_scenarios
      WINNING_COMBOS.each do |set|
        if spaces[set[0]] == spaces[set[1]] &amp;&amp; spaces[set[1]] == spaces[set[2]]
          return true unless spaces[set[0]].nil?
        end
      end
      false
    end

    def tie
      if !spaces.include?(nil) &amp;&amp; !winning_scenarios
        return true
      end
    end
  end

  class Game &lt; Board
    attr_reader :player1, :player2, :symbol
    def initialize
      super
      play_game
    end

    def play_game
      @player1 = Player.new("X")
      @player2 = Player.new("O")
      puts Board.new
      @current_turn = 1
      turn
      win_message
      tie_message
      play_again
    end

    def move(player)
      while !winning_scenarios &amp;&amp; !tie
        puts "Where would you like to move 'player #{player.symbol}'?".red
        choice = gets.chomp.to_i
        check_space(choice, player.symbol)
        puts "Player #{player.symbol}'s move:".green
        puts self
        turn
      end
    end

    def tie_message
      puts "It's a Draw!".cyan if tie
    end

    def win_message
      puts "Game over!".cyan if winning_scenarios
    end

    def turn
      @current_turn.even? ? move(@player2) : move(@player1)
    end

    def play_again
      puts "Play again? (yes or no)".yellow
      answer = gets.chomp.downcase
      if answer == "yes"
        TicTacToe::Game.new
      else
        puts "Goodbye".cyan.bold
      end
    end
  end
end

TicTacToe::Game.new
Not bad.

Unbounded recursion

Inside the Game class:


def initialize
  # ...
  play_game
end

def play_game
  @player1 = Player.new("X")
  @player2 = Player.new("O")
  # ...
  play_again
end

def play_again
  # ...
  if answer == "yes"
    TicTacToe::Game.new
  else
    # ...
  end
end



You're using recursion to repeat the game, which can get arbitrarily deep, leading to a stack overflow error. Use a loop instead. Also, to avoid violating the Single Responsibility Principle, put this loop outside the Game class whose sole responsibility should be to play a game.

Don't call play_again in play_game, and move play_again outside the class. The main code should be:

def play_again?
  puts "Play again? (yes or no)".yellow
  answer = gets.chomp.downcase
  return answer == "yes"
end

loop do
  TicTacToe::Game.new
  unless play_again?
    puts "Goodbye".cyan.bold
    break
  end
end


(I renamed play_again to play_again?. It's conventional to use a question mark in names of methods that return a boolean)

Unbounded recursion 2


def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end

def move(player)
  while !winning_scenarios &amp;&amp; !tie
    # ...
    turn
  end
end



This recursion is not bounded either (it's not bounded by the number of spaces in the board, because a function call occurs for (rejected) invalid moves too). Use a loop instead:

def play_game
  # ...
  while !winning_scenarios &amp;&amp; !tie
    turn
  end
  # ...
end

def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end

def move(player)
  # ...
  # Don't call turn here
end


Object oriented design


class Board; ...; end

class Game &lt; Board; ...; end



A game is not a board. A game has a board. Use composition instead of inheritance:

class Game # NOTE: Don't inherit Board
  def initialize
    @board = Board.new # NOTE
    play_game
  end

  def play_game
    # ...
    while !@board.winning_scenarios &amp;&amp; !@board.tie
      turn
    end
    # ...
  end

  def move(player)
    # ...
    space_available = @board.check_space(choice, player.symbol)
    @current_turn += 1 if space_available
    puts "Player #{player.symbol}'s move:".green
    puts @board # NOTE
  end

  def tie_message
    ... if @board.tie
  end

  def win_message
    ... if @board.winning_scenarios
  end

  # ...
end


Note I moved @current_turn += 1 to Game. A game has a current turn, a board doesn't. Modify check_space so it returns a boolean.

Encapsulation

Make internal methods private, so they aren't visible outside the class:

class Board 
  def initialize; ...; end
  def to_s; ...; end
  def check_space(cell, sym); ...; end
  def winning_scenarios; ...; end
  def tie; ...; end

  private

    WINNING_COMBOS = [
      [0, 1, 2], [3, 4, 5], [6, 7, 8],
      [0, 3, 6], [1, 4, 7], [2, 5, 8],
      [0, 4, 8], [2, 4, 6]
    ]

    def place_symbol(cell, sym); ...; end
      spaces[cell] = sym
    end
end

class Game
  def initialize; ...; end
  def play_game; ...; end

  private

    def move(player); ...; end
    def tie_message; ...; end
    def win_message; ...; end
    def turn; ...; end
end


Remove this line which exposes internal fields of Game: (you never actually use Game#symbol, by the way)

attr_reader :player1, :player2, :symbol


And this line from Board: (then change every occurence of spaces in Board to @spaces)

attr_reader :spaces


Board methods


def check_space(cell, sym)
  if @spaces[cell].nil?
    place_symbol(cell, sym)
  else
    puts "Space unavailable! Please select another cell"
  end
end




Single Responsibility Principle: Board should not do any printing, as it's not its core responsibility. It's better to return a boolean instead and let Game do the printing.
Naming: This method both checks if the space is free and places a symbol. Rename it to place_symbol_if_free.
Naming 2: Use position instead of cell to be consistent with other methods.
Consider splitting this to two methods: space_free?(position) and place_symbol(position, sym) (already exists) and then call both from Game.



WINNING_COMBOS = [
  [0, 1, 2], [3, 4, 5], [6, 7, 8],
  [0, 3, 6], [1, 4, 7], [2, 5, 8],
  [0, 4, 8], [2, 4, 6]
]



Put this inside winning_scenarios, the only method where it's used.


def winning_scenarios
  WINNING_COMBOS.each do |set|
    if @spaces[set[0]] == @spaces[set[1]] &amp;&amp; @spaces[set[1]] == @spaces[set[2]]
      return true unless @spaces[set[0]].nil?
    end
  end
  false
end




Rename winning_scenarios to game_won?. Or to winner and return the winner's symbol
Use Array#any? instead of looping.
Use Array#map to obtain the symbols at the locations of a set, instead of repeatedly accessing @spaces.


Result: 

def winner
  WINNING_COMBOS.any? do |set|
    symbols = set.map { |position| @spaces[position] }
    if symbols[0] == symbols[1] &amp;&amp; symbols[1] == symbols[2]
      symbols[0]
    end
  end
end



def tie
  if !@spaces.include?(nil) &amp;&amp; !winning_scenarios
    return true
  end
end



Rename to tie?, and simply return the boolean:

def tie?
  return !@spaces.include?(nil) &amp;&amp; !winner
end


I also suggets extracting first part to a new private method full?.

to_s can be simplified by using functional programming. See my implementation of as_string in this answer.

Game methods

play_game


puts Board.new



Change this to puts @board.


while !@board.winner &amp;&amp; !@board.tie?



Looks like Board is missing a game_over? method.


win_message
tie_message



These method names are misleading. Each one conditionally prints a message. Merge them to a single print_game_result method.

turn


def turn
  @current_turn.even? ? move(@player2) : move(@player1)
end



The call to move is duplicated because the method does two things: (1) determine who's turn it is (2) call move.

It's better to do only the first one:

def current_player
  @current_turn.even? ? @player2 : @player1
end


Then use move(current_player) in play_game.

move


def move(player)
  puts "Where would you like to move 'player #{player.symbol}'?".red
  choice = gets.chomp.to_i
  space_available = @board.check_space(choice, player.symbol)
  @current_turn += 1 if space_available
  puts "Player #{player.symbol}'s move:".green
  puts @board
end



I'd move the last two puts statements to play_game, because they're not part of making a move.

If you split check_space as I suggest earlier, you can make a loop that gets input until Board#space_free? returns true and then call Board#place_symbol. Otherwise, you'll have to rename this method to try_make_a_move.

initialize

Don't call game_play. Constructors should only initialize an object, and should not do IO. Instead, call game_play in the main code (TicTacToe::Game.new.play_game).

Further Improvements


Say who won.
Use digits 1 to 9 when printing the board and getting user input. It's hard to tell 0 and O apart.