TicTacToe game in Ruby

Question

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 
 attr_reader :board, :place
 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

Answer 1

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

A first modified version:

class Board 
 attr_reader :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:

class Board attr_reader :board

 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).