3
\$\begingroup\$

Board.h

#pragma once
#include <vector>
#include <iostream>
namespace Logs {
    inline void announce_winner(char winner) {

        std::cout << "Game Over! " << winner << " wins the game\n";
    }

    inline void draw() {
        std::cout << "Game Over! .. Draw\n";
    }

    inline void already_chosen() {
        std::cout << "The cell is already chosen, please try again.\n";
    }
    inline void prompt_to_play() {
        std::cout
            << "Please enter a number between 1 and 9 inclusive that is not chosen\n";
    }
    inline void invalid_cell() {
        std::cout << "Invalid choice. ";
        prompt_to_play();
    }

    inline void current_turn(char player_symbol) {
        std::cout << "It's " << player_symbol << " turn\n";
    }

    inline void another_game() {
        std::cout << "Do you want to play another game? Y - N\n";
    }

};
class Board {
private:
    size_t width, height;
    // number of Xs minus number of Os, if +3 X wins, -3 O wins
    std::vector<short> difference_row;
    std::vector<short> difference_col;
    std::vector<short> difference_diagonal;
    unsigned moves;
    std::vector<std::vector<char>> board;
    char player_symbol;
public:
    Board();
    bool is_full();

    void make_move();

    void draw();
    void reset();
    size_t get_width();
    size_t get_height();
    bool is_game_over();
    void increase_moves();
    bool is_valid_move(int row, int col);

    void update_cell(int row, int col);

    void toggle_player();

    bool any_winner();

    char get_symbol() {
        return player_symbol;
    }

};

Board.cpp

#include "Board.h"

Board::Board() { Board::reset(); }

void Board::make_move() {
  draw();
  Logs::current_turn(player_symbol);
  Logs::prompt_to_play();
  int choice;
  std::cin >> choice;

  int row_index = (choice - 1) / height;
  int col_index = (choice - 1) % width;
  if (is_valid_move(row_index, col_index)) {
    update_cell(row_index, col_index);
    increase_moves();
    if (is_game_over()) {
      draw();

      if (any_winner()) {
        Logs::announce_winner(player_symbol);
      } else {
        Logs::draw();
      }
      Logs::another_game();
      std::string res;
      std::cin >> res;
      if (tolower(res[0]) == 'y') {
        reset();
        make_move();
      } else {
        exit(0);
      }
    }
  } else {
    Logs::invalid_cell();
  }
  make_move();
}

bool Board::is_full() { return moves == width * height; }

void Board::draw() {
  for (const auto &row : board) {
    for (const auto &cell : row) {
      std::cout << cell << " ";
    }
    std::cout << std::endl;
  }
}

void Board::reset() {
  width = 3;
  height = 3;
  board.assign(height, std::vector<char>(width));
  char current = '1';
  for (auto &row : board) {
    for (auto &cell : row) {
      cell = current++;
    }
  }
  difference_row = std::vector<short>(3);
  difference_col = std::vector<short>(3);
  difference_diagonal = std::vector<short>(2);
  moves = 0;
  player_symbol = 'X';
}

size_t Board::get_height() { return height; }

size_t Board::get_width() { return width; }

bool Board::is_game_over() { return Board::any_winner() || Board::is_full(); }

void Board::increase_moves() {
  moves++;
  // don't toggle player unless game is over, this will help us to announce
  // winner
  if (!is_game_over()) {
    toggle_player();
  }
}

bool Board::is_valid_move(int row, int col) {
  return !(row < 0 || row >= height || col < 0 || col >= width ||
           board[row][col] == 'X' || board[row][col] == 'O');
}

void Board::update_cell(int row, int col) {
  board[row][col] = player_symbol;
  int add = (player_symbol == 'X' ? +1 : -1);
  difference_row[row] += add;
  difference_col[col] += add;
  if (row == col) {
    difference_diagonal[0] += add;
  }
  if (row + col == height - 1) {
    difference_diagonal[1] += add;
  }
}

void Board::toggle_player() {
  player_symbol = (player_symbol == 'X' ? 'O' : 'X');
}
bool Board::any_winner() {
  for (int i = 0; i < 3; i++) {
    if (i < 2 && abs(difference_diagonal[i]) == 3) {
      return true;
    }
    if (abs(difference_row[i]) == 3 || abs(difference_col[i] == 3)) {
      return true;
    }
  }
  return false;
}

TicTacToe.h

#pragma once
#include "Board.h"

class TicTacToe {
private:
    Board board;
public:
    TicTacToe();
    void start();
};
    

TicTacToe.cpp

#include "TicTacToe.h"

TicTacToe::TicTacToe() {
    board = Board();
}
void TicTacToe::start() {
    while (true) {
        board.make_move();
        if (board.any_winner()) {
            Logs::announce_winner(board.get_symbol());

        }
        else if (board.is_full()) {
            Logs::draw();
        }

    }
}

main.cpp

#include "TicTacToe.h"

int main() {
    TicTacToe tic;
    tic.start();
}

I've tried my best to code a multiplayer tic tac toe console program. Here's what I've written. Please, I need a code review, if there's any modifications, advices, etc. to make the code better.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

General Observations

It is very nice to see a program without using namespace std;. The implementation in main.c is good, nice and short., however, it may be too short. It might be better to have a try{} catch(CONDITION) {} that returns the proper exit status rather than having a call to exit() in void Board::make_move(). The recursive nature of make_move() for a new game may also be a problem. This recursion indicates that the objects could be partitioned better.

There are 2 functions get_width() and get_height() in the Board class that aren't used. this indicates that they aren't needed and should not have been implemented. If they are needed, they are simple enough that they can be implemented in Board.h rather than Board.cpp.

It isn't clear why logging was created as a name space rather than a class. What is clear is that logging should have its own header file, keep every piece of the program as simple as possible.

When you compile C or C++ it is best to use the -Wall switch to provide warning for code that could possibly fail. The following line in Board.cpp generates a warning with my compiler:

    int row_index = (choice - 1) / height;

because height is defined as size_t and row_index is defined as int. It is best to define all variables used as indexes as size_t. The error message indicates that there might be loss of data due to the difference in size between size_t and int. Since the parameters row and col are used as indexes in bool is_valid_move(int row, int col); and void update_cell(int row, int col); they should be declared as size_t rather than int as well.

Use of the inline Keyword

In the Logs namespace every function is defined with the inline keyword. The inline keyword is depreciated, and is only a recommendation to the compiler that may be ignored. Optimizing compilers will perform inlining of function based on multiple factors with or without the inline keyword. There is really no reason to use this keyword anymore.

Magic Numbers

There are Magic Numbers in Board.cpp function (3 and 2), it might be better to create symbolic constants for them to make the code more readable and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

constexpr size_t BOARD_SIZE = 3;

    ...

void Board::reset() {
    width = BOARD_SIZE;
    height = BOARD_SIZE;
    board.assign(height, std::vector<char>(width));
    char current = '1';
    for (auto& row : board) {
        for (auto& cell : row) {
            cell = current++;
        }
    }
    difference_row = std::vector<short>(BOARD_SIZE);
    difference_col = std::vector<short>(BOARD_SIZE);
    difference_diagonal = std::vector<short>(2);
    moves = 0;
    player_symbol = 'X';
}

    ...

bool Board::any_winner() {
    for (int i = 0; i < BOARD_SIZE; i++) {
        if (i < 2 && abs(difference_diagonal[i]) == BOARD_SIZE) {
            return true;
        }
        if (abs(difference_row[i]) == BOARD_SIZE || abs(difference_col[i] == BOARD_SIZE)) {
            return true;
        }
    }
    return false;
}

Object Oriented Design

I am including this to help you learn more about object oriented design. I think you may also want to learn about design patterns.

SOLID is 5 object orieneted design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.