patternrubyMinor
Another Tic Tac Toe In Ruby
Viewed 0 times
toetictacanotherruby
Problem
I am messing with Ruby a little bit and would like to get some feedback about the Spaghettii below :)
```
Sides = ["X", "O"]
Msg_for_restart = " Starting all over again"
Msg_for_coordinates = "Enter coordinates in the form of x,y"
Msg_for_matrix = "This is how the matrix looks like"
Draw_msg = "The game is draw, try again please."
Rows, Columns = 3, 3
class Player
attr_reader :name, :side
def initialize(name, side)
@name, @side = name, side
end
def to_s
"#{@name} with #{@side} side"
end
end
def initialize_matrix(rows, columns)
Array.new(rows) { Array.new(columns) }
end
def print_matrix(matrix)
puts "----"
matrix.each_with_index do |row_value, i|
row_value.each_with_index do |column_value, j|
print matrix[i][j].to_s + " "
end
puts
end
puts "----"
end
def get_input
$stdin.gets.chomp
end
def get_players_info
puts "Welcome to Tic Tac Toe Game"
puts "Enter the name for the first player: "
first_player_name = get_input
puts "Enter a side to play with "
first_player_side = get_input
if not Sides.map { |s|
s.to_s
}.include? first_player_side
puts "Sorry, only X and O values are allowed" row_border || x_player_input[1].to_i > column_border
matrix[x_player_input[0].to_i][x_player_input[1].to_i]= Sides[0]
if is_player_winning_based_on_rows(x_player.side, matrix) || is_player_winning_based_on_columns(x_player.side, matrix) || is_player_winning_based_on_diagonals(x_player.side, matrix)
puts "Congatulations #{x_player.to_s}, You Won!"
break
end
print_matrix(matrix)
puts "Hello #{y_player.to_s}"
puts Msg_for_coordinates
y_player_input = get_input.split(",")
raise IndexError if y_player_input[0].to_i > row_border || y_player_input[1].to_i > column_bor
```
Sides = ["X", "O"]
Msg_for_restart = " Starting all over again"
Msg_for_coordinates = "Enter coordinates in the form of x,y"
Msg_for_matrix = "This is how the matrix looks like"
Draw_msg = "The game is draw, try again please."
Rows, Columns = 3, 3
class Player
attr_reader :name, :side
def initialize(name, side)
@name, @side = name, side
end
def to_s
"#{@name} with #{@side} side"
end
end
def initialize_matrix(rows, columns)
Array.new(rows) { Array.new(columns) }
end
def print_matrix(matrix)
puts "----"
matrix.each_with_index do |row_value, i|
row_value.each_with_index do |column_value, j|
print matrix[i][j].to_s + " "
end
puts
end
puts "----"
end
def get_input
$stdin.gets.chomp
end
def get_players_info
puts "Welcome to Tic Tac Toe Game"
puts "Enter the name for the first player: "
first_player_name = get_input
puts "Enter a side to play with "
first_player_side = get_input
if not Sides.map { |s|
s.to_s
}.include? first_player_side
puts "Sorry, only X and O values are allowed" row_border || x_player_input[1].to_i > column_border
matrix[x_player_input[0].to_i][x_player_input[1].to_i]= Sides[0]
if is_player_winning_based_on_rows(x_player.side, matrix) || is_player_winning_based_on_columns(x_player.side, matrix) || is_player_winning_based_on_diagonals(x_player.side, matrix)
puts "Congatulations #{x_player.to_s}, You Won!"
break
end
print_matrix(matrix)
puts "Hello #{y_player.to_s}"
puts Msg_for_coordinates
y_player_input = get_input.split(",")
raise IndexError if y_player_input[0].to_i > row_border || y_player_input[1].to_i > column_bor
Solution
Input validation
Input validation with recursion is kind of ugly.
A user could crash the program by repeatedly entering invalid values until the call stack overflows.
I recommend to rewrite the input validation in
Usability
I see a couple of usability issues in
Performance
and then checks if all values are equal to
That's kind of a waste if the first value is not equal to
It would be better to stop iterating immediately in that case.
The same issue in
Opportunities to simplify
Instead of this:
How about this:
Instead of the
you could change
I don't know much ruby, but I don't understand why the
Input validation with recursion is kind of ugly.
A user could crash the program by repeatedly entering invalid values until the call stack overflows.
I recommend to rewrite the input validation in
get_players_info to use a loop instead of recursion.Usability
I see a couple of usability issues in
get_players_info:- From the message "Enter a side to play with " it's not obvious what are the valid values. It would be nice to include them in the message.
- If the second user chooses invalid side, we have to start the input all over again, from the name of player 1, and so on. It would be friendlier to repeat from the last invalid point.
- Why offer the choice of side for the second player at all, when there is only one valid choice?
Performance
is_player_winning_based_on_rows builds an array of values for each row,and then checks if all values are equal to
side.That's kind of a waste if the first value is not equal to
side.It would be better to stop iterating immediately in that case.
The same issue in
is_player_winning_based_on_columns too.Opportunities to simplify
Instead of this:
if not Sides.map { |s|
s.to_s
}.include? first_player_sideHow about this:
if not Sides.include? first_player_sideInstead of the
get_x_player and get_y_player functions,you could change
get_players_info to return player_x, player_y, and call it with:player_x, player_y = get_players_infoI don't know much ruby, but I don't understand why the
rescue statement is necessary in the are_elements_equal function.Code Snippets
if not Sides.map { |s|
s.to_s
}.include? first_player_sideif not Sides.include? first_player_sideplayer_x, player_y = get_players_infoContext
StackExchange Code Review Q#150572, answer score: 4
Revisions (0)
No revisions yet.