patternrubyMinor
TicTacToe game in Ruby
Viewed 0 times
tictactoegameruby
Problem
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.
```
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.
Solution
Gratulation! For a beginner you made a good and clear game.
A first modified version:
I removed the reader
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
I integrated the checks into
There is also a check for wrong values (like letters) and different messages.
I replaced
I integrated the game in a method
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:
You could integrate it to
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.
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.playI 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.
nilwould indicate free fields. This could make some checks easier. But with the initialization the board gets its initial values to be shown inshow_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.
Code Snippets
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.playdef 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.playContext
StackExchange Code Review Q#92201, answer score: 3
Revisions (0)
No revisions yet.