Novice implementation of Tic-Tac-Toe

After reading a lot of tutorials, I was finally able to code my version of a Tic-Tac-Toe game. I would like to have some honest review of the code to see what I can do to better my coding.

```
class Game

attr_reader :board , :marker

def initialize
@board = Array.new(3," "){Array.new(3," ")}
@marker = ["X", "O"]
#array that will be use to track all player move
#and use to find if there's any empty space on the board
@track_move ||= []
end

def create_players
# create both players
@names = []

@player_1 = ask "Please enter the first player name: "
@names.push(@player_1)

@player_2 = ask "Please enter second player name: "
@names.push(@player_2)

space
puts"welcome #{@player_1.upcase} and #{@player_2.upcase}"
print_separator
space

puts "Randomizing who'll start..."
sleep(1)
space
# assign player by calling the player_assigment function that will determine who will start first
player_assigment
space
end

def space
puts"\n"
end

def ask(question, kind="string")
print question + " "
answer = gets.chomp.downcase
answer = answer.to_i if kind == "number"
return answer
end

def print_separator(character="-")
puts character * 49
end

def player_assigment
# merge the names array and symbol array
# with the zip method and return a nested array with player and symbol.
@choice = @names.shuffle.zip(marker)
# iterate over the choice nested array and
# print out each player and their assigned symbol
@assign = @choice.to_h
@assign.each do |player, symbol|
puts "#{player.upcase} symbol is #{symbol}"
end
end

def first
# choose the first array in choice nested array and return the first player and symbol
first = @choice.first
@first_player = first.first
@symbol = first.last
@current = @first_player
end

def second
# choose the second array in choice nested array and return the second player and symbol

When seeking improvement of a piece of code, the first thing is removing repetition:

def mapping(move, symbol)
     case move
        when 0
            if board[0][0]=="X" || @board[0][0] == "O"
               invalid
               make_move
            else
               board[0][0] = symbol
               process_move(@move)
            end

        when 1
            if board[0][1]=="X" || board[0][1] == "O"
               invalid
               make_move
            else
              board[0][1] = symbol
              process_move(@move)
            end

        when 2
            if board[0][2]=="X" || board[0][2] == "O"
               invalid
               make_move
            else
               board[0][2] = symbol
               process_move(@move)
            end

        when 3
            if board[1][0]=="X" || board[1][0] == "O"
               invalid
               make_move
            else
               board[1][0] = symbol
               process_move(@move)
            end

        when 4
            if board[1][1]=="X" || board[1][1] == "O"
               invalid
               make_move
            else
               board[1][1] = symbol
               process_move(@move)
            end

        when 5
            if board[1][2]=="X" || board[1][2] == "O"
               invalid
               make_move
            else
               board[1][2] = symbol
               process_move(@move)
            end

        when 6
            if board[2][0]=="X" || board[2][0] == "O"
               invalid
               make_move
            else
               board[2][0] = symbol
               process_move(@move)
            end

        when 7
            if board[2][1]=="X" || board[2][1] == "O"
               invalid
               make_move
            else
               board[2][1] = symbol
               process_move(@move)
            end

        when 8
            if board[2][2]=="X" || board[2][2] == "O"
               invalid
               make_move
            else
               board[2][2] = symbol
               process_move(@move)
            end

        else 
               invalid
               make_move
    end
end

Just the numbers are changing, the actions are the same. We can take advantage of this to shorten this piece of code.

def mapping(move, symbol)
     input_to_coords = { 0 : [0, 0], ... 8 : [2, 2] } # Fill in the ellipsis
     x, y = input_to_coords[move]
     if board[x][y]=="X" || board[x][y] == "O"
           invalid
           make_move
        else
           board[x][y] = symbol
           process_move(@move)
        end
  end

You can also reduce the repetition in check_win

def check_win?
player = @current
    symbol = @symbol
    if board[0][0] == symbol && 
       board[0][1] == symbol &&
       board[0][2] == symbol
       winner(player)

    elsif
       board[1][0] == symbol && 
       board[1][1] == symbol &&
       board[1][2] == symbol
       winner(player)

    elsif
       board[2][0] == symbol && 
       board[2][1] == symbol &&
       board[2][2] == symbol
       winner(player)

    elsif
       board[0][1] == symbol && 
       board[1][1] == symbol &&
       board[2][1] == symbol
       winner(player)

    elsif
       board[0][2] == symbol && 
       board[1][2] == symbol &&
       board[2][2] == symbol
       winner(player) 

    elsif
       board[0][2] == symbol && 
       board[1][1] == symbol &&
       board[2][0] == symbol
       winner(player) 

    elsif
       board[0][0] == symbol && 
       board[1][1] == symbol &&
       board[2][2] == symbol
       winner(player)

    end
     return false
end

If at least one (any?) of the triplets are equal to the given symbol, a player wins otherwise false.

def check_win?
     triplets =  [ [[0,0], [0,1], [0,2]] ... ]
     triplets.any? {|t| t.all? {|x, y| @board[x][y] == @symbol}} ? winner(player) : false
end

Actually I would have chosen a bool as return value, not winner_player

About move_left?

def move_left?
     sum  = @track_move.inject{|r, s| r + s}
     if sum == 36
       return true
     end
end

• sum is unecessary as an intermediate variable.

• inject can be written more shortly.

• if is not needed, nor return is

:

def move_left?
     @track_move.inject(:+) == 36
 end

StackExchange Code Review Q#102233, answer score: 6