Connect4 game multiplayer

Question

I made a connect4 game in c++ please tell my how I can make the code better, thanks.

I already asked some people and they told me i should be using fixed witdh ints and [[nodiscard]] on functions that cant be discarded and also to use noexcept on functions that dont throw

#include <algorithm>
#include <array>
#include <cstdint>
#include <iostream>
#include <utility>

class Board {
public:
  enum class CellType { player1, player2, empty };
  using board_type = std::array<std::array<CellType, 7>, 6>;

private:
  board_type m_board{};
  std::pair<uint8_t, uint8_t> m_latestHit{};

  std::string getCellColor(CellType type) const noexcept {
    switch (type) {
    case CellType::player1:
      return "\033[1;33m O\033[0m"; // red
    case CellType::player2:
      return "\033[1;36m O\033[0m"; // blue
    case CellType::empty:
      return "\033[1;30m O\033[0m"; // black
    }
  }

public:
  Board() {
    for (uint8_t cols{0}; cols < 7; ++cols) {
      for (uint8_t rows{0}; rows < 6; ++rows) {
        m_board[rows][cols] = CellType::empty;
      }
    }
  }

  void reset() noexcept {
    std::for_each(m_board.begin(), m_board.end(), [](auto &col) {
      std::for_each(col.begin(), col.end(),
                    [](auto &cell) { cell = CellType::empty; });
    });
  }

  void pruint8_t() const noexcept {
    std::for_each(m_board.begin(), m_board.end(), [&](auto &col) {
      std::cout << '\t';
      std::for_each(col.begin(), col.end(),
                    [&](auto &cell) { std::cout << getCellColor(cell); });
      std::cout << '\n';
    });
  };

  void hit(uint8_t col, CellType player) noexcept {
    // the first cell on the column that is not already hit
    uint8_t freeCell{static_cast<uint8_t>(getColDepth(col, m_board) - 1)};
    m_board[freeCell][col] = player;
    m_latestHit = {freeCell, col};
  };

  // returns how many cells are not empty in that column
  [[nodiscard]] static constexpr uint8_t
  getColDepth(uint8_t col, const board_type &board) noexcept {
    uint8_t depth{};
    // start loop at top of col
    for (uint8_t row{0}; row < 6; ++row) {
      if (board[row][col] != CellType::empty)
        return row;
    }
    // if all are empty return 6
    return 6;
  };

  [[nodiscard]] board_type getBoard() const { return m_board; };

  class Check {
    board_type board_h{};
    Board::CellType player_h;
    uint8_t m_row{};
    uint8_t m_col{};

  public:
    Check(Board::CellType player, const board_type &board, uint8_t row,
          uint8_t col)
        : player_h{player}, board_h{board}, m_row(row), m_col{col} {};

    [[nodiscard]] bool vertical() noexcept {
      // if row is too low to be able to win return false
      if (m_row >= 3)
        return false;
      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row + 1][m_col] != player_h)
          return false;
      };
      return true;
    };

    [[nodiscard]] bool horizontal() noexcept {

      // if center column check for win on both sides
      if (m_col == 3) {
        return left() || right();

        // check only right side
      } else if (m_col < 3) {
        return right();

        // check noly left side
      } else if (m_col > 3) {
        return left();
      }
      // default
      return false;
    };

    [[nodiscard]] bool left() noexcept {

      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row][m_col - i] != player_h)
          return false;
      }
      return true;
    };

    [[nodiscard]] bool right() noexcept {

      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row][m_col + i] != player_h)
          return false;
      };
      return true;
    };

    [[nodiscard]] bool diagonal() noexcept {
      // for every direction check if the next 4 are equal
      // loop continues until a bad value is found
      // if no bad value is found during the loop return true

      // diagonal right down
      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row + i][m_col + i] == player_h) {
          if (i == 3)
            return true;
          else
            continue;
        } else
          break;
      };

      // diagonal left down
      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row + i][m_col - i] == player_h) {
          if (i == 3)
            return true;
          else
            continue;
        } else
          break;
      };

      // diagonal left up
      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row - i][m_col - i] == player_h) {
          if (i == 3)
            return true;
          else
            continue;
        } else
          break;
      }

      // diagonal right up
      for (uint8_t i = 0; i < 4; ++i) {
        if (board_h[m_row - i][m_col + i] == player_h) {
          if (i == 3)
            return true;
          else
            continue;
        } else
          break;
      }

      // if no wins return false
      return false;
    }
  };

  [[nodiscard]] bool checkForWin(CellType player) noexcept {
    uint8_t row{m_latestHit.first};
    uint8_t col{m_latestHit.second};
    Check check{player, m_board, row, col};
    return (check.diagonal() || check.horizontal() || check.vertical());
  };
};

class Player {
  std::string m_name{};
  Board::CellType m_player{};

public:
  Player(std::string name = "", Board::CellType player = Board::CellType::empty)
      : m_name{name}, m_player{player} {};

  [[nodiscard]] Board::CellType getPlayer() const { return m_player; };

  std::string getName() const { return m_name; };
};

class Game {

private:
  Board board{};
  Player m_p1{};
  Player m_p2{};
  std::array<Player, 2> players{m_p1, m_p2};

  [[nodiscard]] uint8_t getInput() noexcept {
    std::cout << "Enter column:\n";
    uint8_t col;
    std::cin >> col;
    return col;
  }

public:
  Game(Player p1, Player p2) : m_p1{p1}, m_p2{p2} {};

  [[nodiscard]] Player play() {
    uint8_t currentPlayer{};
    uint8_t rounds{};
    uint8_t maxRounds{42};

    while (true) {
      if (rounds == maxRounds)
        throw "TIE!";
      board.pruint8_t();

      uint8_t col;
      while (true) {
        col = getInput();
        // if column is full
        if (Board::getColDepth(col, board.getBoard()) != 1)
          break;

        std::cout << "column is full!\n";
      }
      board.hit(col, players[currentPlayer].getPlayer());

      if (board.checkForWin(players[currentPlayer].getPlayer())) {
        board.pruint8_t();
        return players[currentPlayer];
      }
      currentPlayer = (currentPlayer == 1 ? 0 : 1);
    }
  }
};

[[nodiscard]] bool playAgain() noexcept {
  std::cout << "Restart game? [y/n]\n";
  char x;
  std::cin >> x;
  return (toupper(x) == 'Y');
}

[[nodiscard]] std::string getName(uint8_t nth) noexcept {
  std::cout << "Player " << nth << " name:\n";
  std::string name{};
  std::cin >> name;
  return name;
}

int main() {

  do {
    try {

      Player p1{getName(1), Board::CellType::player1};
      Player p2{getName(2), Board::CellType::player2};
      Game game{p1, p2};

      Player winner{game.play()};
      std::cout << "\nWinner: " << winner.getName() << '\n';
    } catch (char *tie) {
      std::cout << tie;
    }
  } while (playAgain());
};

Answer 1

Line by line analysis

using board_type = std::array<std::array<CellType, 7>, 6>;

There are two literals here, while they should probably be parameters during construction, or otherwise at least constants.

---

return "\033[1;33m O\033[0m"; // red

I don't understand this string type, but it isn't a color. The fact that you need to use an end of line comment says it all really. A constant that describes this string type would be much better.

---

std::pair<uint8_t, uint8_t> m_latestHit{};

Something like a position is really more descriptive than a pair of uints, even if it is just a typedef.

---

for (uint8_t cols{0}; cols < 7; ++cols) {

Three remarks:

• I'd prefer cols = 0 over cols{0} but this might be best practice now (why?);
• the literal 7 is repeated here, you can find out the size of the board, right? Easier if you'd have used parameters or constants;
• ++cols would be considered faster, but for me cols++ is just more readable, I guess by now every C++ programmer knows that ++cols will not immediately be executed, but I hope that they also know that this kind of optimization is unnecessary.

---

void reset() noexcept {
  std::for_each(m_board.begin(), m_board.end(), [](auto &col) {
    std::for_each(col.begin(), col.end(),
                  [](auto &cell) { cell = CellType::empty; });       });
  }
}

I don't see why you need two implementations to set all the cells to empty, just call a (private) method from the constructor and from the public reset method.

Personally I don't think these two for each statements are as readable as the two for loops in the constructor, but hey, maybe this is best practice now.

---

void pruint8_t() const noexcept {

Really don't know what pruint8_t means.

---

[&](auto &cell) { std::cout << getCellColor(cell); });

Ah, seemingly this is supposed to run on a specific terminal, the code / comments should reflect that.

---

void hit(uint8_t col, CellType player) noexcept {

Since when is there a hit in Connect4?

---

getColDepth(uint8_t col, const board_type &board) noexcept {

Note that you already have something about color in there too, so spelling Column out only costs 3 additional characters but spares us a lot of confusion.

---

Game(Player p1, Player p2) : m_p1{p1}, m_p2{p2} {};

End of code, you're getting slightly sloppy; that should be player1 and player2 of course.

---

uint8_t maxRounds{42};

This identical to the number of cells on the board, so Board could return that value by calculating totalRows * totalColumns.

---

throw "TIE!";

Don't ever throw unless something is amiss. A tie doesn't mean the game is broken.

---

  uint8_t col;

I like that; no need to initialize it, so you don't.

---

if (board.checkForWin(players[currentPlayer].getPlayer())) {

The board has become an actor, not just a data class. That's not necessarily bad or anything, but maybe it is something to think about.

Remarks

I like the fact that you only check from the latest piece, and that you have ways of avoiding too many checks. I don't think it matters performance wise - this will take microseconds if not nanoseconds after all - but it is good programming practice to only perform as many operations as strictly necessary.

I do strongly wonder what happens if you place a piece in the middle of a horizontal or diagonal line though. That check doesn't seem present in your code. I also think that having 4 checks should be enough (horizontal, vertical, and the two diagonals). First find the start from one side, then move to the other side from the starting piece, counting the pieces.

---

What is a "Check" class? Usually classes don't use a verb, they use a noun. If you have a class that can perform checks then it should probably be called a Checker instead.

I would think that a Connect4Checker.check(Board board, Position position) would be much nicer (the player can be directly retrieved from the position on the board, there should be a piece there after all).

---

I would have used a different coordinate system, with the left bottom being (0, 0). That way the board can grow to any size without restrictions, and without changing the rules that you've programmed.

If you have a Position class then you can also have a north, west, east and south defined. One good thing is that you can then do neighbor = position.west(1), which can greatly simplify reading the code for checking if there is a connect4. I did this with a chess game. It can also have a print statement which greatly helps debugging.

---

All in all you've got reasonable identifiers, the code does certainly not strike me as badly formatted. White space is reasonable, line size too. Pleasure to read through the code really, not bad at all.