# Plays a game of Tic Tac Toe

As part of the odin project I've written this bit of code that allows two users to play a game of Tic Tac Toe. I'm still a relative newbie to Ruby and to coding and am guessing that there are much better ways to do what I've done here. I'd love to get input, suggestions and recommendations.

The full code is below but can also be found here.

class TicTacToe

def initialize
@game = Game.new("Chris", "Lisa")
end

class Game
attr_accessor :player1, :player2

def initialize (player1, player2)
@player1 = Player.new(player1)
@player2 = Player.new(player2)
@winning = [[1,2,3], [4,5,6], [7,8,9], [1,4,7], [2,5,8], [3,6,9], [1,5,9], [3,5,7]]
@symbol1 = "X"
@symbol2 = "O"

self.start
end

def start
introduction
player_names
player_assignment
display_board(@winning)
play_game
end

def introduction
puts "Welcome to TicTacToe. We will now randomly choose who goes first. Press any key to begin."
puts ""
@@input = gets.chomp
end

def player_names
puts "Please enter the name of a player."
name1 = gets.chomp
puts ""
puts "Please enter the name of another player."
name2 = gets.chomp

@player1.name = name1
@player2.name = name2
end

def player_assignment
@x = rand(10)

if @x <= 5
@player1.symbol = @symbol1
@player2.symbol = @symbol2
puts ""
puts "#{player1.name} is assigned #{@symbol1} and #{player2.name} is assigned #{@symbol2}"
puts ""
else
@player1.symbol = @symbol2
@player2.symbol = @symbol1
puts ""
puts "#{player1.name} is assigned #{@symbol2} and #{player2.name} is assigned #{@symbol1}"
puts ""
end
end

def play_game
count = 0
if @player1.goes_first? == true
while count < 8
check_board(@player1.play, @player1.symbol)
announce_winner(is_winner?(@player1))
count += 1

check_board(@player2.play, @player2.symbol)
announce_winner(is_winner?(@player2))
count += 1
end
else
while count < 8
check_board(@player2.play, @player2.symbol)
announce_winner(is_winner?(@player2))
count += 1

check_board(@player1.play, @player1.symbol)
announce_winner(is_winner?(@player1))
count += 1
end
end
end_game
end

def display_board(array)
puts ""
print "  #{array[0][0]}  |   #{array[0][1]}  |   #{array[0][2]} \n"
puts "------------------"
print "  #{array[1][0]}  |   #{array[1][1]}  |   #{array[1][2]} \n"
puts "------------------"
print "  #{array[2][0]}  |   #{array[2][1]}  |   #{array[2][2]}"
puts "\n"
end

def check_board(move, symbol)
@move = move.to_i
@symbol = symbol

@winning.each do |array|
array.map! do |num|
if num.is_a?(String)
num
elsif(@move == num)
@symbol
elsif @move != num && num.is_a?(Integer)
num
end
end
end
display_board(@winning)
puts ""
end

def is_winner?(player)

@winning.any? do |line|
line.all? {|position| position == player.symbol}
end
end

def announce_winner(x)
@x = x

if @x == true
puts "You've won, congratulations"
display_board(@winning)
exit
else
return
end
end

def end_game
puts "GAME OVER"
exit
end
end

class Player
attr_accessor :name, :symbol

def initialize(name)
@name = name
end

def play
puts "#{@name}, your symbol is #{self.symbol}."
puts "Please select a square represented."
input = gets.chomp
puts "#{@name} selected #{input} and a #{symbol} will be placed in that position."
puts ""
return input
end

def goes_first?
if self.symbol == "X"
return true
else
return false
end
end
end
end

a = TicTacToe.new

• Not a real answer, but what will you do when the requirement or need arises to change the size of the board? say to 4x4 or 5x5? In that case you will have to make several changes to your code (displaying, winning conditions, etc.). – David Perfors Apr 8 '15 at 10:34

As Caridorc points out, the code is readable overall. I do however have some concerns:

• Typically, Ruby style guides will say that you should use 2 spaces for indentation

• Why is the game initialized with "Chris" and "Lisa" as players, when it never uses that for anything? Given that the game asks for players anyway, it shouldn't be necessary to pass anything to Game.new.

• I'd suggest making TicTacToe a module instead, just for namespacing purposes. There's little need for it to be a class.

• Your method names are a little off.

• #announce_winner doesn't necessarily announce a winner
• #check_board doesn't check the board
• #is_winner? should probably just be #winner? ("is_" isn't necessary when you've got the question mark)
• #player_names doesn't return player names, as its name would suggest as it's a noun.
• #player_assignment is also a noun, but should be a verb, e.g. shuffle players, since that's what it does.
• I'm very confused as to why #introduction creates a class variable called @@input. There's no reason to store the user's input. Nor is there a reason to chomp it.
... and "press any key to begin" isn't quite true: You have to press enter.

• Also of note are the @symbol1 and @symbol2 variables, which don't seem to have any reason to be instance variables in Game. In general, you seem to use instance variables when you should use local variables.

• You don't need most - if not all - of the returns in the code.

• As Caridorc points out, there's quite a few instances of code duplication. Often for dealing with either @player1 or @player2.

• The #start method is very clear in making the steps of the game explicit, but the #play_game method is more muddled. You always call #announce_winner.. even if there's nothing to announce.

• Speaking of #announce_winner: Why create an instance variable to store an argument variable? Why have an else branch that does nothing?

• I wouldn't let the Player be responsible for getting input from the user. It splits the responsibility for interacting with the user between Game and Player Doubly so, since using that input for anything happens in Game.

• As mentioned, you have a bunch of instance variables that shouldn't be instance variables, but for some reason, the actual state of the game - the board - is not an instance variable. Instead, you pass an array around from method to method. It's almost the opposite of how object-oriented programming usually works.

• You can keep picking already-claimed squares... Probably not what you want.
It doesn't change the board, but it makes you forfeit your turn. You can keep picking, say, 5 for both players, and after 9 turns, the game just ends, even though there are 8 unclaimed squares on the board.
You can also just type any random thing other than 1-9, and the game will let you. Again, you just forfeit your turn.
In short, you should really make sure that user's input makes sense before continuing with the game.

Here's a very different implementation (I did it mostly for fun). I'm not saying this is "the right way™" (there are things I'm not happy with, as you can see in the comments). It's just to present an alternative.

# A tiny mixin for Array
class Array
# Get every nth element in the array
def every_nth(step)
0.step(self.count, step).map { |i| self[i] }
end
end

module TicTacToe
# Convenience method
def self.play
Game.new.play
end

class Game
def initialize
@board = Board.new
show_welcome_message
create_players
take_turn until @board.full?
puts "It's a tie!"
end

def show_welcome_message
puts "Welcome to TicTacToe. Press enter to begin."
gets
end

def create_players
names = []
print "Please enter the name of the first player: "
names << gets.chomp # TODO: Should check that something was actually entered

print "Please enter the name of the second player: "
names << gets.chomp # TODO: See above...

puts "Randomizing who'll start..."

@players = names.shuffle.zip(["X", "O"]).map do |name, symbol|
Player.new(symbol, name)
end
end

def print_board
puts @board
end

# Note: I'm not too happy with this method. It's a little messy and recursive...
def take_turn
print_board
puts "It's #{current_player.name}'s (#{current_player.symbol}) turn."
square = get_player_choice
@board[square] = current_player.symbol
check_board # this may exit

puts ""
@players.rotate! # swap players around for next round
end

def current_player
@players.first
end

def get_player_choice
begin
print "Enter the number of an unclaimed square: "
square = gets.chomp.to_i
end until @board.unclaimed_squares.include?(square)
square
end

def check_board
if @board.won?(current_player.symbol)
puts "#{current_player.name} wins!"
exit
end
end
end

# The Player class is simply a Struct
Player = Struct.new(:symbol, :name)

# The Board class handles the state of play, checking win conditions etc.
# I'm not quite happy with checking for Fixnums or Strings - a bit too informal...
class Board

def initialize
@squares = (1..9).to_a
end

def won?(symbol)
winning_squares.any? do |line|
line.all? { |value| value == symbol }
end
end

def full?
@squares.all? { |square| square.is_a?(String) }
end

def []=(number, value)
@squares[number-1] = value
end

def unclaimed_squares
@squares.select { |square| square.is_a?(Fixnum) }
end

def to_s
lines = rows.map do |row|
row.join(" | ")
end
lines.join("\n#{'-' * 9}\n")
end

private

def winning_squares
rows + columns + diagonals
end

def rows
@squares.each_slice(3).to_a
end

def columns
rows.transpose
end

def diagonals
diagonals = []
diagonals << @squares.every_nth(4) # top-left to bottom-right
diagonals << @squares.reverse.every_nth(4) # the other one
diagonals
end
end
end

TicTacToe.play


I can simplify your player_assignment:

• You used a random variable up to ten and then checked if it was less than 5, you can use a random boolean instead

• You repeated very similar code two times, I removed that duplication

def player_assignment

if rand(2) == 1
@player1.symbol = @symbol1
@player2.symbol = @symbol2
else
@player1.symbol = @symbol2
@player2.symbol = @symbol1
end

puts ""
puts "#{player1.name} is assigned #{@player1.symbol} and #{player2.name} is assigned #{@player2.symbol}"
puts ""
end


Also as a simple tip do not overcheck things that are already booleans

def goes_first?
if self.symbol == "X"
return true
else
return false
end
end


should become:

def goes_first?
return self.symbol == "X"
end


I am pretty sure that there is some duplication to be removed down here but I can't understand how:

    if @player1.goes_first? == true
while count < 8
check_board(@player1.play, @player1.symbol)
announce_winner(is_winner?(@player1))
count += 1

check_board(@player2.play, @player2.symbol)
announce_winner(is_winner?(@player2))
count += 1
end
else
while count < 8
check_board(@player2.play, @player2.symbol)
announce_winner(is_winner?(@player2))
count += 1

check_board(@player1.play, @player1.symbol)
announce_winner(is_winner?(@player1))
count += 1
end
end


You should not use explicit while loops with count,

while count < 8


should become

(1..9).each do |turns_played|


removing the increment.

Overall the code is very clearly written, kudos.

• @rand_bool = rand(2); if @rand_bool... This does not work, rand(2) always returns a truthy value (0 or 1). You need to have a smarter if condition; try if rand(2) == 1 and get rid of @rand_bool entirely (which, by the way, shouldn't be an instance var) – Devon Parsons Apr 7 '15 at 19:09
• @DevonParsons so 0 is truthy, I am always learning, thanks. – Caridorc Apr 7 '15 at 19:12
• Yup, only nil and false are falsey - everything is truthy. By the way, the goes_first? method can be reduced to just symbol == "X". The return is implicit, and the self is necessary either :) – Flambino Apr 7 '15 at 21:24