Tic-Tac-Toe & counter against the player's moves and count scores

Here's a Tic Tac Toe program I wrote in C++. It can play (semi-effectively) against the player. Please give tips on how to improve.

```
#include
#include
#include
#include

using namespace std;

int computerRandomPick; //random location which computer selects if first.
int computerPick; //used to decide computer vs player AI moves.
//player interaction
int playerChoice;
bool playerTurn;
int gameWin = 3;
//gameWin is integer because it will allow program to explicitly say who wins instead of bool which will allow you to only declare game state.

//board arrays
char blockOne = '1';
char blockTwo = '2';
char blockThree = '3';
char blockFour = '4';
char blockFive = '5';
char blockSix = '6';
char blockSeven = '7';
char blockEight = '8';
char blockNine = '9';
//random starting turn chooser
int turnFirst; //variable to decide whoever goes first

int checkWinComputer()
{
if(blockOne == 'O' && blockFive == 'O' && blockNine == 'O' && playerTurn == false) //diagonal, 1 - 5 = 9
gameWin = 2; //This will make computer win.
if(blockThree == 'O' && blockFive == 'O' && blockSeven == 'O' && playerTurn == false) //diagonal, 3 - 5 = 7
gameWin = 2;
if(blockOne == 'O' && blockTwo == 'O' && blockThree == 'O' && playerTurn == false) //horizontal 1 - 2 = 3
gameWin = 2;
if(blockFour == 'O' && blockFive == 'O' && blockSix == 'O' && playerTurn == false) //horizontal 4 - 5 = 6
gameWin = 2;
if(blockSeven == 'O' && blockEight == 'O' && blockNine == 'O' && playerTurn == false) //horizontal 7 - 8 = 9
gameWin = 2;
if(blockOne == 'O' && blockFour == 'O' && blockSeven == 'O' && playerTurn == false) //vertical 1 - 4 = 7
gameWin = 2;
if(blockTwo == 'O' && blockFive == 'O' && blockEight == 'O' && playerTurn == false) //vertical 2 - 5 = 8
gameWin = 2;
if(blockThree == 'O' && blockSix == 'O' && blockNine == 'O' && playerTurn == false) //vertical 3 - 6 = 9
gameWin = 2;
}
int checkWinPlayer()
{
if(blockOn

As Loki Astari points out, there is a complete Tic Tac Toe strategy. However, I'm going to ignore that and just review the code that you posted. The main reason is that I think that a code review is more productive if you can compare code that at least follows the same algorithm.

I would get rid of the global variables and replace them with class variables.

The gameWin variable might better be an enum than an int. The value of the enum is self-commenting. With an int, you need to remember what each value means. Same thing for turnFirst, although you could also just make that a boolean playerFirst.

I would replace the nine block variables with a single array. Something like

const int ROW_SIZE = 3;
const int COLUMN_SIZE = 3;
const int BOARD_SIZE = ROW_SIZE * COLUMN_SIZE;
char blocks[BOARD_SIZE+1] = "123456789";

Why check if playerTurn is false/true in the check functions? If it's necessary, you could just do

if ( ! playerTurn ) {  // == false
    return NO_WIN; // assuming NO_WIN is one of the enum values
}
if ( playerTurn ) {  // == true
    return NO_WIN;
}

but it doesn't seem necessary.

The two check functions could easily be one checkWin() function. Also, you could add checkRow() and checkColumn() functions to reduce the copy and paste code.

bool checkColumn(char *column, char toCheck) {
    for ( int rowStart = 0; rowStart < BOARD_SIZE; rowStart += ROW_SIZE ) {
        if ( column[rowStart] != toCheck ) {
            return false;
        }
    }

    return true;
}

bool checkWin(char toCheck) {
    // the first row starts at the same place as the board
    // the subsequent rows start ROW_SIZE blocks after the last
    for ( int offset = 0; offset < BOARD_SIZE; offset += ROW_SIZE ) {
        if ( checkRow(blocks + offset, toCheck) ) {
          return true;
        }
    }

    // the first column starts the same place as the board
    // subsequent columns start one more
    for ( int offset = 0; offset < ROW_SIZE; offset++ ) {
        if ( checkColumn(blocks + offset, toCheck) ) {
          return true;
        }
    }

    // both diagonals must include the center block
    if ( blocks[4] != toCheck ) {
        return false;
    }

    if ( blocks[0] == toCheck && blocks[8] == toCheck ) {
        return true;
    }

    if ( blocks[6] == toCheck && blocks[2] == toCheck ) {
        return true;
    }

    return false;
}

You return 0 from computerAI() but you never use the result. Just make it a void. You don't need to return at the end, only if you want to abort early. Same thing with checkPlayerInput().

The checkPlayerInput() function should probably be called isInputValid() and can be made shorter:

bool isInputValid(int playerChoice) {
    if ( playerChoice == blocks[playerChoice-1] ) {
        blocks[playerChoice-1] = playerLetter;
        return true;
    }

    return false;
}

It's odd to use a global variable for playerChoice. You'd usually just pass it into the functions that needed it.

In your computerAI() function, why not just return once a move has been found rather than setting the playerTurn boolean? Something like

void computerAI() {
    if ( computerLetter == blocks[0] && computerLetter == blocks[1] && '3' == blocks[2] ) {
        blocks[2] = computerLetter;
        status = COMPUTER_WIN;
        return;
    }

Also note that X is always the player that goes first in standard Tic Tac Toe rules. So if the player can sometimes go second, then the player should sometimes be O.

The computer's move should not depend on the player's choice but on the state of the board.

In the computerAI() function, you have a do while loop on code that should only be run once. Fortunately, I think that it will only run once, but there is still no reason to enclose it in a do while. That code only runs if the player did not choose 3, 6, or 9, which seems like a bug.

You don't have to check for a computer win separately. You can do it as part of generating the computer's move.

You never checkComputerInput(), so get rid of that function.

You can check for ties with a for loop:

bool checkTie() {
    for ( int offset = 0; offset < BOARD_SIZE; offset++ ) {
        // if any block is still numbered as its count, then there are moves left
        if ( '1' + offset == blocks[offset] ) {
            return false;
        }
    }

    return true;
}

Even easier, just check if the block is any number. Or remember that a tie will always occur after the BOARD_SIZE move. Tracking a counter is easier than checking each block after every move.

There's no reason for the computer to pick its first move randomly. Picking the center square is the optimal move.

Almost everything in the main function should be moved into a method (e.g. play) on an object. One, this will provide access to the previously global variables that are now part of the object. Two, this wil

StackExchange Code Review Q#66578, answer score: 9