Non-AI Tic-Tac-Toe program

Question

I'm fairly new to C++ and to game programming itself. Today I decided to build a multiplayer Tic Tac Toe program using only elemental C++ syntax/data structures. I also implemented a text interface for the game instead of real graphics.

Any suggestions or tips for the code?

#include <iostream>
#include <string>

#define GRID_SIZE 9

using namespace std;

void printGame(char gameGrid[]);
void inputToSlot(char gameGrid[], int slot, char choice);
void initialiseGame(char gameGrid[]);
bool isFull(char gameGrid[]);
bool isEmpty(char input);
bool checkWon(char gameGrid[]);
bool rowWon(char gameGrid[]);
bool columnWon(char gameGrid[]);
bool diagonalWon(char gameGrid[]);

int main(int argc, const char * argv[])
{
    char gameGrid[GRID_SIZE];
    initialiseGame(gameGrid);
    int currentTurn = 0;

    cout << "Tic Tac Toe by Bryan Lean" << endl << endl;
    printGame(gameGrid);
    cout << endl;

    while (!isFull(gameGrid)) {
        int slotChoice;
        currentTurn += 1;
        bool isP2 (currentTurn % 2 == 0);

        // Prompt
        if (isP2) {
            cout << "Player 2 turn..." << endl << "Insert to slot > ";
        } else {
            cout << "Player 1 turn..." << endl << "Insert to slot > ";
        }
        cin >> slotChoice;

        while (!isEmpty(gameGrid[slotChoice])) {
            cout << "Slot occupied. Please select another slot > ";
            cin >> slotChoice;
        }

        // Insert
        if (isP2) {
            gameGrid[slotChoice] = 'o';
        } else {
            gameGrid[slotChoice] = 'x';
        }

        // Print
        printGame(gameGrid);
        cout << endl;

        // Check Winner
        if (checkWon(gameGrid)) {
            if (isP2) {
                cout << "Player 2 won the game.";
            } else {
                cout << "Player 1 won the game.";
            }

            break;
        } 

    }

    if (!checkWon(gameGrid)) {
        cout << "Draw.";
    }


}

void initialiseGame(char gameGrid[]) {
    for (int i = 0; i < GRID_SIZE; ++i) {
        gameGrid[i] = ' ';
    }
}

void printGame(char gameGrid[]) {
    cout << "+---+---+---+" << endl;
    for (int i = 0; i < GRID_SIZE; ++i) {
        cout << "| " << gameGrid[i] << " ";
        if ((i+1) % 3 == 0 && i != 0) {
            cout << "|" << endl;
            cout << "+---+---+---+" << endl;
        }
    }
}

void inputToSlot(char gameGrid[], int slot, char choice) {
    gameGrid[slot] = choice;

}

bool isFull(char gameGrid[]) {
    for (int i = 0; i < GRID_SIZE; ++i) {
        if (gameGrid[i] == ' ') {
            return false;
        }
    }

    return true;
}

bool isEmpty(char input) {
    if (input == ' '){
        return true;
    }
    return false;
}

bool checkWon(char gameGrid[]) {
    if (rowWon(gameGrid)) {
        return true;
    } else if (columnWon(gameGrid)) {
        return true;
    } else if (diagonalWon(gameGrid)) {
        return true;
    }

    return false;
}

bool rowWon(char gameGrid[]) {
    for (int i = 0; i < GRID_SIZE; i += 3) {
        char firstInRow = gameGrid[i];
        char secondInRow = gameGrid[i + 1];
        char thirdInRow = gameGrid[i + 2];

        if (!isEmpty(firstInRow) && !isEmpty(secondInRow) && !isEmpty(thirdInRow)) {
            if (firstInRow == secondInRow && firstInRow == thirdInRow) {
                return true;
            }
        }
    }

    return false;
}

bool columnWon(char gameGrid[]) {
    for (int i = 0; i < 3; ++i) {
        char firstInColumn = gameGrid[i];
        char secondInColumn = gameGrid[i + 3];
        char thirdInColumn = gameGrid[i + 6];

        if (!isEmpty(firstInColumn) && !isEmpty(secondInColumn) && !isEmpty(thirdInColumn)) {
            if (firstInColumn == secondInColumn && firstInColumn == thirdInColumn) {
                return true;
            }
        }

    }

    return false;
}

bool diagonalWon(char gameGrid[]) {
    char center = gameGrid[4];
    if (!isEmpty(center) && !isEmpty(gameGrid[0]) && !isEmpty(gameGrid[8])) {
        if (center == gameGrid[0] && center == gameGrid[8]) {
            return true;
        }
    } else if (!isEmpty(center) && !isEmpty(gameGrid[2]) && !isEmpty(gameGrid[6])) {
        if (center == gameGrid[2] && center == gameGrid[6]) {
            return true;
        }
    }

    return false;

}


/************************

 Tic Tac Toe
Text Graphics

 +---+---+---+
 | x | o | x |
 +---+---+---+
 | x | o | x |
 +---+---+---+
 | o | o | x |
 +---+---+---+

*************************/

Answer 1

Overall, I think it is a pretty good program.

I think I see one bug. You don't check that the user's input for slotChoice is valid. If the input is outside the range 0..8, the program overruns the array.

Some style and refactoring suggestions, in no particular order...

• I don't like the global GRID_SIZE constant. I would make a struct (or a class) that holds gameGrid and the size. Then pass a reference to the struct (a const reference when possible) to the functions that currently have a gameGrid parameter.

• Later, if you want to make the program more object-oriented, you can make most of the current functions into member functions of the class. Since you are new to C++, I would treat that as a separate project.

• Consider putting currentTurn in the struct also.

• I think getting the slotChoice input and validating it is a good bit of work to move to a new function. I would move this:

  cin >> slotChoice;
  
  while (!isEmpty(gameGrid[slotChoice])) {
      cout << "Slot occupied. Please select another slot > ";
      cin >> slotChoice;
  }
  

  plus new code to check that the input is in range, to a new function.

• (Small point) I like to read things in their natural order. I'd rather see the "stuff" for player 1 come before the "stuff" for player 2. So I'd use a bool named isP1 and rearrange several conditional statements.

• In the "Check Winner" section of the while loop, you could save the return value from checkWon() in a local bool variable. Then you wouldn't need to call the function again to decide if the game was a draw.

• I see some code duplication, especially in the code that generates output.

  cout << "Player 2 won the game."; appears again, with only 1 character changed.

  cout << "+---+---+---+" << endl; appears twice, and could go in a separate function.

• An alternative for checkWon():

  bool checkWon(const char gameGrid[]) {
     if (rowWon(gameGrid)) {
         return true;
     }
     if (columnWon(gameGrid)) {
         return true;
     }
     if (diagonalWon(gameGrid)) {
         return true;
     }
  
     return false;
  }
  

  I made the parameter const. I changed the cascading else if statements to three independent if statements. I think that reflects the underlying logic a bit better; the three win conditions aren't mutually exclusive.

• You could simplify the logic in function rowWon(). If you check isEmpty(firstInRow), you don't need to also check isEmpty(secondInRow) and isEmpty(thirdInRow). (The later comparisons with firstInRow will exclude cases where the other cells in the row are empty.) columnWon() and diagonalWon() could get the same simplification.

• There's a lot of code duplication (and conceptual duplication) between RowWon(), columnWon(), and diagonalWon(). At the core of each is a complicated if statement that examines 3 cells and returns a bool result. But all of those if statements are essentially the same. That decision could go in a a separate function -- pass it the 3 cell values. The other part of each function involves deciding which 3 cells to test; those decisions are different for each win condition.

Answer 2

The code already looks nice. It is easy to understand and well structured. I would change these things:

• You define GRID_SIZE as 9. This made me immediately wonder: Why not 3?, since Tic Tac Toe is played on a 3 times 3 board. Usually the 'board size' is measured in one dimension only.

• Defining this constant is probably useless, unless you want to switch to other board sizes later.

• But if you define this constant, you should change all the other code so that by just changing this constant you can immediately play on a 9 times 9 board. Currently there are lots of 3 in the code.

• The line bool isP2 (currentTurn % 2 == 0); looks unusual. Simple variables are usually initialized using the assigment operator, like this: bool isP2 = (currentTurn % 2 == 0);.

• Before checking isEmpty(gameGrid[slotChoice]) you should make sure that 0 <= slotChoice && slotChoice < GRID_SIZE. Otherwise you are invoking 'undefined behaviour'.

• You should move the // Check winner code out of the while loop. The loop should then be while (!checkWon(gameGrid) && !isFull(gameGrid)) { ... }.

• In the expression (i+1) % 3 == 0 && i != 0 you can leave out the i != 0, since that can never be true.

• The function inputToSlot is defined but not used.

• In the function isEmpty you can simply write:

  bool isEmpty(char input) {
      return input == ' ';
  }
  

• In the functions rowWon, columnWon and diagonalWon you don't need to check for isEmpty(secondInRow). It is enough to check !isEmpty(firstInRow) && secondInRow == firstInRow && thirdInRow == firstInRow.

Answer 3

Here are some things that could help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Use const variables instead of #define

The use of #define to define constants in C++ is generally better avoided. Instead, for example, your code could have used const size_t GRID_SIZE = 9;

Avoid spurious use of std::endl

The code uses std::endl in a number of places where it's not useful to do so. Remember that std::endl inserts a newline character and flushes the stream. If you don't really need to flush the stream, which is the case for nearly every instance in this program, you should simply use the \n character instead.

Use constants consistently

Your code creates a constant GRID_SIZE, which suggests that it might be possible to change it to some other value, but then the printGrid() function has a hardcoded 3 which make that not really work. With just a bit more effort, your code could be modified to accept any size square, or even a rectangular shape if you were interested in that.

Reconsider the definition of isEmpty()

Right now your code has a whole function isEmpty which is actually identical in function to input==' ' which is rather slim for a function. Further, every instance it's used, it's actually passed a copy of the contents of one of the squares in gameGrid. What would make more sense is to pass in the index value into isEmpty and avoid making all of those copies.

Reconsider the definition of checkWon()

Since you already know, based on isP2, which player might just have won, it would make your code simpler if you would pass that value to checkWon() so that it could avoid checking for empty squares. All the code really needs to look for in that instance is either an o or an x value.

Use objects

Your code defines several functions that all refer to the same gameGrid array. This strongly suggests that you could (and should!) use a class instead and make those functions member functions of the class.

Hide arbitrary implementation choices

If you use an object, the specific characters used for x, o and empty should all be private members of the class. That way, you could arbitrarily change to some other value and calling code would need not know of the change.

Scrub input data

Right now, you use an inserter to get the value of slotChoice but if the user inputs a letter, the code will be stuck in an infinite loop. Also, you really must add bounds checking to disallow the user putting in some value such as 209581 and altering memory well outside the program space.

update ...

I refactored your code into a C++ class:

class TicTacToe 
{
private:
    // stores the actual square game grid contents
    char *gameGrid;
    // width and total number of squares in the gameGrid
    size_t width, squares; 
    // horizontal bar to print between rows
    std::string hbar;
    // which player's turn is it? (0 or 1) 
    int player;
    // number of free squares remaining on game board
    unsigned freeslots;
    // this contains one character each for player 1, player 2 and empty
    static const char pchar[3];
public:
    // create a square TicTacToe board with width = n
    TicTacToe(size_t n=3);
    // destructor
    ~TicTacToe();
    // reset can be used to play again after object exists
    void reset();
    // inserter for printing board state to ostream
    friend std::ostream &operator <<(std::ostream &out, const TicTacToe &ttt);
    bool isNotFull() const { return freeslots > 0; }
    // return character representing the player whose turn it is
    char playerTurn() const { return pchar[player]; }
    // return character representing player who just took a turn
    char lastPlayer() const { return pchar[1-player]; }
    // return true if the slot was successfully marked for current player
    bool applyTurn(unsigned slot);
    // return true if the last player just won
    bool winner() const;
};

As you can see, the counting of turns has disappeared, replaced by a single private member player which keeps track of which player is next to play. You might want to start with this template and see if you can fill in the missing member functions based on the code you've already written.

Answer 4

The others have mentioned many great things, but I'll add some thing that were left out:

• You can shorten the code a bit by defining main() at the very bottom, thus you will no longer need that long list of function prototypes.

• Use storage containers instead of C-arrays in C++. C-arrays can be especially problematic when being passed to functions because they decay to pointers; the array itself isn't actually passed. Storage containers are also safer and are more useful because they come with functions.

  For this program, you may use std::vector or, if you have C++11, std::array. There are many resources around on how to use storage containers effectively.

• Instead of using += for 1:

  currentTurn += 1;
  

  just use ++ (pre or post increment):

  currentTurn++;
  

• A simple conditional like this:

  bool isEmpty(char input) {
      if (input == ' '){
          return true;
      }
      return false;
  }
  

  can be shortened to this:

  bool isEmpty(char input) {
      return input == ' ';
  }
  

  This will still automatically return true or false.

• Instead of manually filling an array with a loop:

  for (int i = 0; i < GRID_SIZE; ++i) {
      gameGrid[i] = ' ';
  }
  

  use std::fill_n:

  std::fill_n(gameGrid, GRID_SIZE, ' ');
  

  This is part of <algorithm> and it works with both C-arrays and certain storage containers that support the appropriate iterators.

Answer 5

For a better user experience, instead of printing empty spaces in the board, print the number of that square as a placeholder:

void initialiseGame(char gameGrid[]) {
    for (int i = 0; i < GRID_SIZE; ++i) {
        gameGrid[i] = '1'+i;
    }
}

That would require some corresponding adjustments elsewhere:

bool isFull(char gameGrid[]) {
    for (int i = 0; i < GRID_SIZE; ++i) {
        if (gameGrid[i] != 'x' && gameGrid[i] != 'o') {
            return false;
        }
    }

    return true;
}

bool isEmpty(char input) {
    if (input != 'x' && input != 'o'){
        return true;
    }
    return false;
}

Users will frequently want to play more than one round, so you should ask whether they want to play again.

cout<<"Do you want to play again\n";
string s;
cin>>s;
if(s=="yes")
    main();

---

When printing output to cout, you should consistently terminate the line with \n.