Simple terminal TicTacToe

Question

Coming from JavaScript, I needed to learn C++ recently, so I thought a great way to start is to create a simple terminal-based tic-tac-toe game. I encountered some difficulties especially with multi-dimensional arrays. I got it to work, but I don't think it was done very elegantly. I also experimented with the OOP C++ provides. Here's my code:

main.cpp

#include <stdio.h>
#include <stdlib.h>

#include <iostream>
#include <string>

#include "colors.h"
#include "term.h"
using namespace std;

void playerNames(string &p1, string &p2) {
  cout << "Enter name of player 1 (O): ";
  getline(cin, p1);
  cout << "Enter name of player 2 (X): ";
  getline(cin, p2);
}

class Game {
 public:
  Game(string p1, string p2) {
    clear();
    this->p1 = p1;
    this->p2 = p2;
  };
  int play() {
    while (1) {
      if (this->moves == 9 && !won) {
        printBoard();
        return -1;
      }
      printBoard();
      input();
      if (won) {
        printBoard();
        break;
      }
    }
    return !turn;
  }

 private:
  string p1, p2;
  bool turn = 1;
  bool won = 0;
  int moves = 0;
  int win[8][3][2] = {{{0, 0}, {0, 1}, {0, 2}}, {{1, 0}, {1, 1}, {1, 2}},
                      {{2, 0}, {2, 1}, {2, 2}}, {{0, 0}, {1, 0}, {2, 0}},
                      {{0, 1}, {1, 1}, {2, 1}}, {{0, 2}, {1, 2}, {2, 2}},
                      {{0, 0}, {1, 1}, {2, 2}}, {{0, 2}, {1, 1}, {2, 0}}};
  char board[3][3] = {{' ', ' ', ' '}, {' ', ' ', ' '}, {' ', ' ', ' '}};
  void printBoard(int r = 1, int k = 1) {
    auto printInstructions = [&]() {
      if (turn == 1) {
        cout << p1 << "'s (O) turn";
      } else {
        cout << p2 << "'s (X) turn";
      }
      cout << endl
           << "Use the arrow keys to move, press enter to confirm your choice";
    };
    clear();
    string nl[3][3];
    for (int u = 0; u < 3; ++u) {
      nl[u][0] = " " + string{board[u][0]} + " ";
      nl[u][1] = " " + string{board[u][1]} + " ";
      nl[u][2] = " " + string{board[u][2]} + " ";
    }
    nl[r][k] = BRE + nl[r][k] + RST;
    cout << nl[0][0] << "|" << nl[1][0] << "|" << nl[2][0] << endl;
    cout << "-----------" << endl;
    cout << nl[0][1] << "|" << nl[1][1] << "|" << nl[2][1] << endl;
    cout << "-----------" << endl;
    cout << nl[0][2] << "|" << nl[1][2] << "|" << nl[2][2] << endl;
    cout << endl << endl;
    printInstructions();
    cout << endl << endl;
  };
  bool testWin() {
    const int winSize = 8;
    char player = turn ? 'O' : 'X';
    for (int i = 0; i < winSize; i++) {
      bool cond = (board[win[i][0][0]][win[i][0][1]] == player) &&
                  (board[win[i][1][0]][win[i][1][1]] == player) &&
                  (board[win[i][2][0]][win[i][2][1]] == player);
      if (cond) return true;
    }
    return false;
  };
  void input() {
    int pos[2] = {1, 1};
    char c;
    while (1) {
      c = arrow();
      if (c == '\n') {
        if (board[pos[0]][pos[1]] != ' ') {
          continue;
        }
        board[pos[0]][pos[1]] = turn ? 'O' : 'X';
        break;
      }
      if (c == '\0') continue;
      switch (c) {
        case 'u':
          pos[1] = pos[1] == 0 ? 0 : pos[1] - 1;
          break;
        case 'd':
          pos[1] = pos[1] == 2 ? 2 : pos[1] + 1;
          break;
        case 'l':
          pos[0] = pos[0] == 0 ? 0 : pos[0] - 1;
          break;
        case 'r':
          pos[0] = pos[0] == 2 ? 2 : pos[0] + 1;
          break;
      }
      printBoard(pos[0], pos[1]);
    };
    if (testWin()) won = 1;
    moves++;
    turn = !turn;
  };
};

int main() {
  clear();
  cout << "Welcome to TicTacTerm!" << endl;
  cout << "Enter the player details below. You can exit at any time by presing "
          "ESC."
       << endl
       << "Use the arrow keys to navigate, press enter to confirm." << endl;
  string p1, p2;
  playerNames(p1, p2);
  cout << endl << endl;
  Game game(p1, p2);
  int winner = game.play();
  if (winner == -1) {
    cout << "Game over. TIE!" << endl;
  } else if (winner) {
    cout << "Game over. " << p1 << " [O] won!" << endl;
  } else {
    cout << "Game over. " << p2 << " [X] won!" << endl;
  }
  cout << "Press enter to exit..." << endl;
  cin.ignore();
  return 0;
}

term.h

#include <conio.h>

#define KEY_UP 72
#define KEY_DOWN 80
#define KEY_LEFT 75
#define KEY_RIGHT 77
#define KEY_ENTER 13
#define KEY_ESC 27

#ifdef _WIN32
void clear() { system("cls"); }
void home() { printf("\x1b[H"); }
char arrow() {
  int c = _getch();
  if (c == KEY_ESC) exit(0);  // in case you want to exit the program...
  switch (c) {
    case KEY_UP:
      return 'u';
    case KEY_DOWN:
      return 'd';
    case KEY_LEFT:
      return 'l';
    case KEY_RIGHT:
      return 'r';
    case KEY_ENTER:
      return '\n';
    default:
      return '\0';
  }
}
#else
throw "Only Windows OS supported right now.";
#endif

colors.h

#ifndef _COLORS_
#define _COLORS_

#define RST "\x1B[0m"
#define KRED "\x1B[31m"
#define KGRN "\x1B[32m"
#define KYEL "\x1B[33m"
#define KBLU "\x1B[34m"
#define KMAG "\x1B[35m"
#define KCYN "\x1B[36m"
#define KWHT "\x1B[37m"

#define BYEL "\u001b[43m"
#define BRE "\u001b[48;5;240m"

#define FRED(x) KRED x RST
#define FGRN(x) KGRN x RST
#define FYEL(x) KYEL x RST
#define FBLU(x) KBLU x RST
#define FMAG(x) KMAG x RST
#define FCYN(x) KCYN x RST
#define FWHT(x) KWHT x RST

#define BOLD(x) "\x1B[1m" x RST
#define UNDL(x) "\x1B[4m" x RST

#endif

Screenshot

Difficulties I couldn't solve:

• I wasn't able to get the length of the multidimensional array (with arr.size() or arr.length())... it seems to throw an error. So I just hard coded the length values
• I'm not very experienced with other OS's, so this is platform specific to Windows
• I didn't want to hard code the length of the multi-dimensional arrays, but I couldn't find a way to do it (trying to achieve something like int arr[][][] = {{{...}, {...}}, {{...}, {...}}, {{...}, {...}}}; instead of int arr[3][3][2] = {...};)
• It seems like when the user is going to input arrow keys to move, Ctrl+C didn't work in terminating the program (on PowerShell at least), so I just manually implemented an exit with Esc

I'm also not sure how "right", i.e. proper my implementation is. Any suggestions are welcome, gratefully.

Answer 1

Looking at the headers first, I see that term.h is missing include guards, and colors.h is using a reserved identifier for its include guard (any symbol beginning with _ followed by uppercase letter is reserved for any purpose - my advice is to avoid starting identifiers with underscore at all).

I'm not a fan of hard-coded escape sequences, but I don't know Windows - perhaps it only supports one kind of terminal? That may well become an issue if you ever want to support more capable OSes. And weirdly, we depart and use an external process for clearing screen, which is even more problematic - why not use the platform's escape sequence for that, which at least keeps things consistent, and makes for easier porting (e.g. to termcap or curses).

term.h definitely needs the platform-specific #include within the #ifdef WIN32. And the bit we don't reach would be better with a preprocessor #error rather than an out-of-place throw. The former will halt compilation immediately, rather than continuing to parse and emit further errors and warnings.

term.h needs appropriate includes for std::system, std::printf and std::exit rather then depending on those headers already being included - as a rule, the implementation file should include your own headers first, to expose such omissions.

---

Moving on to the implementation file, using namespace std; is a bad habit you should shake off as soon as possible.

Always examine input operations carefully - don't ignore the return value from std::getline (unless you've set the input stream to throw exceptions on failures and errors).

Instead of assigning in the constructor (this->p1 = p1), use an initializer list to set members:

  Game(string p1, string p2)
    : p1{p1},
      p2{p2}
  {
    clear();
  };

I'm not convinced that merely constructing a Game object should have a side-effect of clearing the screen. That's going to make unit-tests difficult, and prevents re-using this in a game server, for example.

The win array can be const, and I think the player names can too (once their initialisation is fixed as above).

board might be better as a flat array of size 9. We can interpret that as rows and columns when we access it, perhaps using a simple function:

char& square(int x, int y) {
    return board[3 * y + x];
}

That makes storing coordinates (e.g. in win) much easier, as there's then only a single value to store. Consider using std::array rather than plain C-style arrays, too.

std::endl is pointless when we go on immediately to print more output. When there's no need to flush the stream, just use ordinary newline ('\n').

---

There's more to look at, but this review is getting a bit long. I recommend addressing the issues we have here and posting a follow-up question for further input.

Answer 2

Use a curses library

It's great that you already thought about portability, and put platform-specific code in term.h and colors.h. However, I recommend that instead of dealing with all the details of terminal output yourself, you use a library that takes care of things for you. Use a curses library, like PDCurses for Windows and ncurses for almost all other platforms. These libraries all have a common API, so you don't need any platform-specific handling in your own code.

Avoid calling system()

Even if you don't want to use a curses library, avoid using system() to do things like clearing the screen. Calling system() will spawn a new shell process, which in turn will have to parse the command you passed it, and depending on the command it might have to spawn yet another process. Apart from having a huge overhead, the behavior of shell commands can change based on environment variables, and they are almost always non-portable.

Since you are already using ANSI color codes, you can just print the ANSI control sequence to clear the screen.