Ruby CLI TicTacToe

Question

I have done Tictactoe game in Ruby. It is my first Object Oriented project in Ruby. I would like to separate all program into a few classes such as Player Board and Game. I want to use more OOP best practices.
Also, my move method is looking so big. I would like to refactor my move method because the code is repeating. In a game class, there is 2 break if code piece. Is there any way to reduce it? Lastly, In my module class, do you think that everything is okay or should I use a more concise way?
And also a total review would be great! Thank you for now.

Here is the code. It is working.

module CheckWinner
  def check_horizontal(player1, player2)
    player1_win = false
    player2_win = false
    @board.each do |row|
      player1_win = row.all?(player1)
      player2_win = row.all?(player2)
      break if player1_win || player2_win
    end
    puts "#{player1} won!" if player1_win
    puts "#{player2} won!" if player2_win
    player1_win || player2_win
  end

  def check_vertical(player1, player2)
    player1_win = false
    player2_win = false
    @board.transpose.each do |row|
      player1_win = row.all?(player1)
      player2_win = row.all?(player2)
      break if player1_win || player2_win
    end
    puts "#{player1} won!" if player1_win
    puts "#{player2} won!" if player2_win
    player1_win || player2_win
  end

  def check_diagonal(player1, player2)
    if @board[0][0] == player1 && board[1][1] == player1 && board[2][2] == player1 ||
       @board[0][2] == player1 && board[1][1] == player1 && board[2][0] == player1
      puts "#{@player1} won!"
      true

    elsif @board[0][0] == player2 && board[1][1] == player2 && board[2][2] == player2 ||
          @board[0][2] == player2 && board[1][1] == player2 && board[2][0] == player2
      puts "#{@player2} won!"
      true
    end
  end
end

# TicTacToe Board
class Board
  include CheckWinner
  attr_accessor :board

  def initialize
    @board = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
  end

  def print_board
    puts '-------------'
    @board.each do |row|
      print '|'
      row.each do |col|
        print " #{col}"
        print ' | '
      end
      puts
    end
    puts '-------------'
  end

  def occupied_error(value)
    puts 'There is a value or wrong place! Try Again!'
    print_board
    value == @player1 ? player2(@player1) : player1(@player2) # Stay same player
  end

  def move(place, value) # Place : 1-9, value = X || Y
    case place
    when 1
      if @board[0][0] == 'X' || @board[0][0] == 'O'
        occupied_error(value)
      else
        @board[0][0] = value
      end
    when 2
      if @board[0][1] == 'X' || @board[0][1] == 'O'
        occupied_error(value)
      else
        @board[0][1] = value
      end
    when 3
      if @board[0][2] == 'X' || @board[0][2] == 'O'
        occupied_error(value)
      else
        @board[0][2] = value
      end
    when 4
      if @board[1][0] == 'X' || @board[1][0] == 'O'
        occupied_error(value)
      else
        @board[1][0] = value
      end
    when 5
      if @board[1][1] == 'X' || @board[1][1] == 'O'
        occupied_error(value)
      else
        @board[1][1] = value
      end
    when 6
      if @board[1][2] == 'X' || @board[1][2] == 'O'
        occupied_error(value)
      else
        @board[1][2] = value
      end
    when 7
      if @board[2][0] == 'X' || @board[2][0] == 'O'
        occupied_error(value)
      else
        @board[2][0] = value
      end
    when 8
      if @board[2][1] == 'X' || @board[2][1] == 'O'
        occupied_error(value)
      else
        @board[2][1] = value
      end
    when 9
      if @board[2][2] == 'X' || @board[2][2] == 'O'
        occupied_error(value)
      else
        @board[2][2] = value
      end
    else
      puts 'It is not a valid value,please try again!(0..9)'
      occupied_error(value)
    end
  end

  def full?
    if @board.flatten.all?(String)
      puts "Draw!"
      true
    end
  end

  def choosing_player
    puts 'Choose for Player1(X or O)'
    loop do
      @player1 = gets.chomp!
      break if @player1 == 'X' || @player1 == 'O'

      puts 'Try Again!(X or O)'
    end
    puts "Player 1 is: #{@player1}"

    @player1 == "X" ? @player2 = "O" : @player2 = "X"
    puts "Player 2 is: #{@player2}"
    print_board
  end

  def player1(player1)
    puts "Choice #{player1} Place on a board(1 to 10)"
    @place = gets.chomp!.to_i
    move(@place, player1)
    print_board
  end

  def player2(player2)
    puts "Choice #{player2} Place on a board(1 to 10)"
    @place = gets.chomp!.to_i
    move(@place, player2)
    print_board
  end

  def game
    choosing_player
    loop do
      player1(@player1)
      break if check_vertical(@player1, @player2) == true || check_diagonal(@player1, @player2) == true || check_horizontal(@player1, @player2) == true || full?

      player2(@player2)
      break if check_vertical(@player1, @player2) == true || check_diagonal(@player1, @player2) == true || check_horizontal(@player1, @player2) == true || full?
    end
  end
end

board = Board.new
board.game

Repl it repo : https://replit.com/@Burakkepuc/TicTacToe#main.rb

Answer 1

Welcome to Code Review, you have almost the same code block repeated 9 times:

  if @board[0][2] == 'X' || @board[0][2] == 'O'
    occupied_error(value)
  else
    @board[0][2] = value
  end

So write a function and use it instead:

def assign_if_possible(x, y, value)
      if @board[y][x] == 'X' || @board[y][x] == 'O'
        occupied_error(value)
      else
        @board[y][x] = value
      end

and use it instead. Also using the % modulo operator should allow you to avoid the case statement altogether, something like (x = val % 3, y = (val / 3).to_i), the exact expression should not be too hard to find.

Answer 2

Simplifying Board referencing will profoundly simplify the using code.

@board = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]

#using actual coordinates
@diagonals = [
               [@board[0][0], @board[1][1], @board[2][2]],
               [@board[0][2], @board[1][1], @board[2][0]], 
             ]
@columns   = [
               [@board[0][0], @board[1][0], @board[2][0]],
               [@board[0][1], @board[1][1], @board[2][1]], 
               [@board[0][2], @board[1][2], @board[2][2]], 
             ]

@rows   =    [
               [@board[0][0], @board[0][1], @board[0][2]],
               [@board[1][0], @board[1][1], @board[1][2]], 
               [@board[2][0], @board[2][1], @board[2][2]], 
             ]

def winner? (player) {  # 'X' or 'O'
  won = false
  won = @[email protected]      { |row|   row.all?(player) ? true : won }
  won = @[email protected] { |diag| diag.all?(player) ? true : won }
  won = @[email protected]   { |col|   col.all?(player) ? true : won }

  won
end #winner?  

---

The program should accept lower case "x" or "o" and change it without making the user re-enter. And same consideration if user enters zero instead of oh. Or is it vice versa?

---

Use integers as Hash keys

Then mapping [[1, 2, 3], [4, 5, 6], [7, 8, 9]] to the board will subdue that awful case statement

@user_entry = {
            1 => @board[0][0],
            2 => @board[0][1],
            3 => @board[0][2]
            # et cetera
          }

Bonus! Now the board can be initialized with nil and still have user enter 1 - 9. A consistent initial value will simplify more code.

def move(player, entry) {
   @user_entry[entry] = player unless occupied?(@user_entry[entry])
end

def occupied?(this_square)
  return @user_entry[this_square].nil? ? false : true
end