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