Tic Tac Toe in Ruby

Question

This 2-player tic-tac-toe game is the first thing I've really written from scratch in Ruby, without the guidance of a tutorial. I'm interested in knowing what could be improved upon to make the code better.

Please point out anything you see that deviates from common Ruby conventions, or that would improve the code in any way.

class Board

  def initialize
    @board = Array.new(3) { Array.new(3, " ") }
  end

  def printInstructions
    puts "1 | 2 | 3",
         "---------",
         "4 | 5 | 6",
         "---------",
         "7 | 8 | 9"
    print "\n"
  end

  def printBoard
    (0..2).each do |row|
      print "       "
      (0..2).each do |col|
        print @board[row][col]
        print " | " unless col == 2
      end
      print "\n"
      print "       ---------\n" unless row == 2
    end
    print "\n"
  end

  def findWinner
    #  X X X        X
    #          &    X
    #               X

    (0..2).each do |i|
      if @board[i][0] == @board[i][1] && @board[i][1] == @board[i][2]
        return @board[i][0] unless @board[i][0] == " "

      elsif @board[0][i] == @board[1][i] && @board[1][i] == @board[2][i]
        return @board[0][i] unless @board[0][i] == " "
      end
    end

    #  X               X
    #    X     &     X
    #      X       X

    if ( @board[0][0] == @board[1][1] && @board[1][1] == @board[2][2] ) ||
       ( @board[0][2] == @board[1][1] && @board[1][1] == @board[2][0] )
      return @board[1][1] unless @board[1][1] == " "
    end

    #  X O X
    #  X O X
    #  O X O

    return "C" unless @board.join.split('').include?(" ")

    # Undecided

    return "U"
  end

  def dropPiece(piece, row, col)
    @board[row][col] = piece if (0..2) === row &&
                                (0..2) === col &&
                                @board[row][col] == " "
  end
end

board = Board.new
active_player = "X"

puts "\n" * 100
board.printInstructions

while board.findWinner == "U"

  puts " #{active_player}'s turn. Choose a box!",
       "        **~V~**"
  print "           "
  move = gets.chomp.to_i - 1
  row = move / 3
  col = move % 3

  puts "\n" * 100

  if board.dropPiece(active_player, row, col)
    if active_player == "X"
      active_player = "O"
    else
      active_player = "X"
    end
  else
    puts "                 Invalid move, please select again\n\n"
  end

  board.printBoard
end

winner = board.findWinner

puts "\n" * 100
puts "   --**~^^^^^^^~**--"

  if winner == "C"
    puts "   C A T S   G A M E"
  else
    puts "     #{winner} ' S   W I N"
  end

puts "   ----**~vVv~**----"
puts "\n"
board.printBoard
puts "\n        **~V~**"

Answer 1

I suggest to divide the dropPiece function in smaller, easy and readable parts as follows:

  def is_empty(row,col)
      @board[row][col] === " "
  end

  def inside_board(row,col)
      (0..2) === row and (0..2) === col
  end

  def valid_move(row,col)
      is_empty(row,col) and inside_board(row,col)
  end

  def dropPiece(piece, row, col) #makeMove
    @board[row][col] = piece if valid_move(row,col)
  end

---

The following piece of code should be

1. Commented or
2. A function with a meaningful name:

:

@board.join.split('').include?(" ")

I would have used:

def isTie
  @board.join.split('').include?(" ")
end

return "C" if isTie

Do not be scared of writing small one-liner functions, they will increibly lower debugging time by improving modularity and readibility.

---

You use the so much puts "\n" * 100 define a function and use that:

def clearScreen
  puts "\n" * 100
end

---

In findWinner you should return false if you have not decided, this allows you to change the code from

while board.findWinner == "U"

to

while not board.findWinner

that is much clearer.

---

The following should be a function called something like alert_winner(winner)

puts "\n" * 100
puts "   --**~^^^^^^^~**--"

  if winner == "C"
    puts "   C A T S   G A M E"
  else
    puts "     #{winner} ' S   W I N"
  end

puts "   ----**~vVv~**----"
puts "\n"
board.printBoard
puts "\n        **~V~**"

---

The following should be a function called promptUser

  puts " #{active_player}'s turn. Choose a box!",
       "        **~V~**"
  print "           "
  move = gets.chomp.to_i - 1
  row = move / 3
  col = move % 3

---

In my opinion the main part should be the shorter possible and only call other functions

---

Minor: you should use and instead of &&, they are exactly the same but the first one is clearer.

---

Putting it all together, here's the improved version(I added a tiny method at the end):

class Board

  def initialize
    @board = Array.new(3) { Array.new(3, " ") }
  end

  def printInstructions
    puts "1 | 2 | 3",
         "---------",
         "4 | 5 | 6",
         "---------",
         "7 | 8 | 9"
    print "\n"
  end

  def printBoard
    (0..2).each do |row|
      print "       "
      (0..2).each do |col|
        print @board[row][col]
        print " | " unless col == 2
      end
      print "\n"
      print "       ---------\n" unless row == 2
    end
    print "\n"
  end

  def isTie
    @board.join.split('').include?(" ")
  end

  def findWinner
    #  X X X        X
    #          &    X
    #               X

    (0..2).each do |i|
      if @board[i][0] == @board[i][1] && @board[i][1] == @board[i][2]
        return @board[i][0] unless @board[i][0] == " "

      elsif @board[0][i] == @board[1][i] && @board[1][i] == @board[2][i]
        return @board[0][i] unless @board[0][i] == " "
      end
    end

    #  X               X
    #    X     &     X
    #      X       X

    if ( @board[0][0] == @board[1][1] && @board[1][1] == @board[2][2] ) ||
       ( @board[0][2] == @board[1][1] && @board[1][1] == @board[2][0] )
      return @board[1][1] unless @board[1][1] == " "
    end

    #  X O X
    #  X O X
    #  O X O

    return "C" unless isTie

    return false
  end

  def is_empty(row,col)
      @board[row][col] === " "
  end

  def inside_board(row,col)
      (0..2) === row and (0..2) === col
  end

  def valid_move(row,col)
      is_empty(row,col) and inside_board(row,col)
  end

  def dropPiece(piece, row, col)
    @board[row][col] = piece if valid_move(row,col)
  end
end

def clear_screen
  puts "\n" * 100
end

def prompt_move(active_player)
  puts " #{active_player}'s turn. Choose a box!",
       "        **~V~**"
  print "           "

  move = gets.chomp.to_i - 1
  row = move / 3
  col = move % 3
  return row,col
end

def alert_winner(winner,board)
  puts "   --**~^^^^^^^~**--"

    if winner == "C"
      puts "   C A T S   G A M E"
    else
      puts "     #{winner} ' S   W I N"
    end

  puts "   ----**~vVv~**----"
  puts "\n"
  board.printBoard
  puts "\n        **~V~**"
end

def ticTacToe(boardClass)
  board = boardClass.new
  active_player = "X"

  clear_screen
  board.printInstructions

  while not board.findWinner

    row,col = prompt_move(active_player)
    clear_screen

    if board.dropPiece(active_player, row, col)
      if active_player == "X"
        active_player = "O"
      else
        active_player = "X"
      end
    else
      puts "                 Invalid move, please select again\n\n"
    end

    board.printBoard
  end

  winner = board.findWinner
  clear_screen

  alert_winner(winner,board)
end

while true
  clear_screen
  puts "Do you want to play again? (y/n)"
  if ["no","n"].include? (gets.chomp.downcase)
    puts "Goodbye"
    break
  end
  ticTacToe(Board)
end

Answer 2

Considering that the number pads on computer keyboards are arranged as

$$\begin{array}{|c|c|c|} \hline 7 & 8 & 9 \\ \hline 4 & 5 & 6 \\ \hline 1 & 2 & 3 \\ \hline \end{array}$$

I would suggest that inverting the grid numbering convention would produce a more pleasant user experience.

Answer 3

2 is an inexplicable magic number for tic-tac-toe. On the other hand, 3 is perfectly expected to appear.

Therefore, instead of writing your ranges as (0..2), you should write (0...3), using the three-dot form to exclude the upper bound.

Answer 4

A quick note: examine these lines in your code.

  if active_player == "X"
    active_player = "O"
  else
    active_player = "X"
  end

This is more succinctly expressed in a ternary, i.e:

active_player = active_player == "X" ? "O" : "X"

That's a direct translation and a much more 'ruby' way of doing it. There's another way to do it as well, but it's not necessarily better. It lends itself nicely to extending the number of players - imagine you later wanted a 4 player tic-tac-toe game.

def next_player(current_player)
  players = ["X", "O", "T", "S"]
  players[1+players.index(current_player)] || players[0]
end

I wouldn't use this with your current requirements, but the possibility is there if you needed the extensibility.