# TicTacToe game in Ruby

I would appreciate any feedback you could offer. I'm self-learning, so I don't really know a "standard" that my code should be up to. I'm also not sure how to get rid of that bit of logic at the bottom, or if it's acceptable.

class Board

def initialize
@board = [1, 2, 3, 4, 5, 6, 7, 8, 9]
puts "Player 1 is X, Player 2 is O."
show_board
end

def show_board
@board.each_slice(3) { |x| puts x.join }
end

def get_place
@place = gets.to_i - 1
end

def free_space?
@board[place].is_a? Integer
end

def put_marker(mark)
get_place
until free_space?
puts "Invalid move, choose again!"
get_place
end
@board[place] = mark
show_board
end

def player1_win_condition
ary = [[1, 2, 3], [4, 5, 6], [7, 8, 9], [1, 4, 7], [2, 5, 8], [3, 6, 9], [1, 5, 9], [3, 5, 7]]
ary.any? do |array|
array.all? { |num| @board[num - 1] == "X" }
end
end

def player2_win_condition
ary = [[1, 2, 3], [4, 5, 6], [7, 8, 9], [1, 4, 7], [2, 5, 8], [3, 6, 9], [1, 5, 9], [3, 5, 7]]
ary.any? do |array|
array.all? { |num| @board[num - 1] == "O" }
end
end

def draw_condition
@board.all? { |num| num.is_a? String }
end

def end_game
if player1_win_condition == true
puts "Player 1 wins!"
exit
elsif player2_win_condition == true
puts "Player 2 wins!"
exit
elsif draw_condition == true && player1_win_condition == false && player2_win_condition == false
puts "It's a draw!"
exit
end
end
end

class Player
attr_accessor :mark

def initialize(mark)
@mark = mark
end
end

player_1 = Player.new("X")
player_2 = Player.new("O")
new_board = Board.new

until new_board.end_game
puts "Player 1 turn"
new_board.put_marker(player_1.mark)
if new_board.end_game != true
puts "Player 2 turn"
new_board.put_marker(player_2.mark)
end
end


Gratulation! For a beginner you made a good and clear game.

A first modified version:

class Board

def initialize(player1, player2)
@board = (1..9).to_a    #You can use also a range to initialize the array
puts "Player 1 is %s, Player 2 is %s." % [player1.mark, player2.mark]
@player_1 = player1
@player_2 = player2
show_board
end

def play

until end_game?
puts "Player 1 turn"
put_marker(@player_1.mark)
if ! end_game?
puts "Player 2 turn"
put_marker(@player_2.mark)
end
end
end

def show_board
puts '-'*9          #I prefer some lines in the board
@board.each_slice(3) { |x| #Good! You use slice.
puts x.join(' | ')
puts '-'*9
}
end

def get_place
while
place = gets.to_i - 1 #Use a return parameter
#Check also if you get only values between 1 and 9 (no letters, no 0)
if ! place.between?(0,9)
puts "Invalid move, use 1..9. Please choose again!"
next
end
if ! free_space?(place)
puts "Invalid move, Pos. %i is occupied. Please choose again!"
next
end
break #stop loop, Value is ok
end
return place
end

def free_space?(place)
@board[place].is_a? Integer
end

def put_marker(mark)
@board[get_place] = mark
show_board
end

def win?(player)
ary = [[1, 2, 3], [4, 5, 6], [7, 8, 9], [1, 4, 7], [2, 5, 8], [3, 6, 9], [1, 5, 9], [3, 5, 7]]
ary.any? do |array|
array.all? { |num| @board[num - 1] == player.mark }
end
end

def draw_condition
@board.all? { |num| num.is_a? String }
end

def end_game?
if win?(@player_1)  #use other check. The comparison wit == true is not necessary.
puts "Player 1 wins!"
show_board  #Show end situation
exit
elsif win?(@player_2)
puts "Player 2 wins!"
show_board  #Show end situation
exit
elsif draw_condition == true && player1_win_condition == false && player2_win_condition == false
puts "It's a draw!"
exit
end
end
end

class Player
attr_accessor :mark

def initialize(mark)
@mark = mark
end
end

new_board = Board.new(Player.new("X"), Player.new("O"))
new_board.play


I removed the reader place and the variable @place. There is no reason why you need it. It is only a temporary used variable, there is no need to store it in the class. I replaced it with local variables and method return parameters.

The use of the return command is not necessary. I used it here to show explicit when I want to return a value.

I integrated the checks into get_place. There is also a check for wrong values (like letters) and different messages.

I replaced player1_win_condition with a generic test with parameter.

I integrated the game in a method play. So it would be easier to start multiple games.

Most important: You used the fix constant 'X' and 'O' in your Game. But this data are also defined a parameter in the class Player. This can make problems if you replace X by x or something else.

What is missing: * There is no check for similar Player.marks

What could be checked/Modified:

• I don't like the end_game? in the actual version. You could integrate it to play (player 1 can only win after his move, same for player 2. Then end_game? needs only to check for draw.
• The board could be a simple array without initialization. nil would indicate free fields. This could make some checks easier. But with the initialization the board gets its initial values to be shown in show_board.

Another version!

The first version had some code repetition. For each player the code was written again.

The new version avoids this repetion:

    def initialize(player1, player2)
@board = (1..9).to_a    #You can use also a range to initialize the array
@player_1 = player1
@player_2 = player2
puts "Player 1 is %s, Player 2 is %s." % [player1.mark, player2.mark]
show_board
end

def play
sequence = [@player_1,@player_2]
until draw?
puts "Player %s turn" % sequence.first.name
put_marker(sequence.first.mark)
if win?(sequence.first)
puts "Player %s wins!" % sequence.first.name
show_board  #Show end situation
return
end
sequence.rotate! #switch to next player
end
puts "It's a draw!" #end message if not left before, becaus eone player wins
end

def show_board
puts '-'*9          #I prefer some lines in the board
@board.each_slice(3) { |x| #Good! You use slice.
puts x.join(' | ')
puts '-'*9
}
end

def get_place
while
place = gets.to_i - 1 #Use a return parameter
#Check also if you get only values between 1 and 9 (no letters, no 0)
if ! place.between?(0,9)
puts "Invalid move, use 1..9. Please choose again!"
next
end
if ! free_space?(place)
puts "Invalid move, Pos. %i is occupied. Please choose again!"
next
end
break #stop loop, Value is ok
end
return place
end

def free_space?(place)
@board[place].is_a? Integer
end

def put_marker(mark)
@board[get_place] = mark
show_board
end

def win?(player)
ary = [[1, 2, 3], [4, 5, 6], [7, 8, 9], [1, 4, 7], [2, 5, 8], [3, 6, 9], [1, 5, 9], [3, 5, 7]]
ary.any? do |array|
array.all? { |num| @board[num - 1] == player.mark }
end
end

def draw?
@board.all? { |num| num.is_a? String }
end

end

class Player
attr_accessor :mark
attr_accessor :name

def initialize(name, mark)
@name = name
@mark = mark
end
end

new_board = Board.new(Player.new('Fred', "X"), Player.new('Joe', "O"))
new_board.play


To identify the players I need also a name. Before you had the fix 'Player one'. You could also remove the text Player in the output (replace Player X turn with X turn).