Single player Tic-Tac-Toe game

Question

I have made an attempt to make a single player mode for the Tic-Tac-Toe game. I am not sure if this is the right way to do it and wanted a second opinion.

    // keeps track of all the valid positions.
bool validPos[10] = {false, true, true, true, true, true, true, true, true, true};
char user = 'O', computer = 'X';

int main(void)
{
    // A two-dimensional array depicting the game.
    char game[3][3] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
    char choice;

    // Asks user for a valid input.
    do
    {
        printf("X or O ?\n");
        scanf(" %c", &choice);

    }while(choice != 'X' && choice != 'O' && choice != 'x' && choice != 'o');

    // assigns X or O to the player.
    if(choice == 'x' || choice == 'X')
    {
        user = 'X';
        computer = 'O';
    }

    display(game);

    printf("You - %c\n", user);
    printf("Computer - %c\n", computer);

    startGame(game, user);
}

Function startGame

void startGame(char game[3][3], const char turn)
{
    int i, j;
    int pos;

    srand((unsigned)time(NULL));
    if(turn == user)
    {
        // asks user for a valid input.
        do
        {
            printf("Your turn : ");
            scanf("%d", &pos);

        }while((pos > 9 || pos < 1) || !validPos[pos]);
    }

    else
    {
        do
        {
            // calculates a good and valid move for the computer.
            pos = rand() % 10;
        }while((pos > 9 || pos < 1) || !validPos[pos]);
        pos = calcMove(game, pos);
    }

    // makes the position chosen invalid.
    validPos[pos] = false;

    // Assigns 'X' or 'O' to the chosen position.
    for( i = 0; i < 3; i++)
        for( j = 0; j < 3; j++)
            if(game[i][j] - '0' == pos)
                game[i][j] = turn;

    // prints the game onto the screen.
    display(game);

    // checks if the player or computer has won or not.
    if(gameStatus(game))
    {
        if(turn == user)
            printf("You Win !\n");

        else
            printf("Computer Wins !\n");

        return;
    }

    // checks if there are any further moves possible.
    // also acts as the base case for the game to end.
    if(isComplete(game))
    {
        printf("Game Over!\nNo More Possible Moves\n");
        return;
    }

    // recursive call to continue the game.
    if(turn == user)
        startGame(game, computer);

    if(turn == computer)
        startGame(game, user);
}

Function calcMove This is where I am unsure about my method of calculating a good move for the computer as I feel this is redundant.

int calcMove(char game[][3], int pos)
{
    int i, j;
    int move = 0;

    // If there is any move along a row that is a winning move for the computer
    // then take it or blocks the winning move of the user along a row.
    for(i = 0; i < 3; i++)
    {
        if(game[i][0] == game[i][1] && validPos[game[i][2] - '0'])
        {
            move = game[i][2] - '0';
            if(game[i][0] == computer)
                return move;
        }

        if(game[i][0] == game[i][2] && validPos[game[i][1] - '0'])
        {
            move = game[i][1] - '0';
            if(game[i][0] == computer)
                return move;
        }

        if(game[i][1] == game[i][2] && validPos[game[i][0] - '0'])
        {
            move = game[i][0] - '0';
            if(game[i][1] == computer)
                return move;
        }
    }

    // If there is any move along a coloumn that is a winning move for the computer
    // then take it or blocks the winning move of the user along a coloumn.
    for(j = 0; j < 3; j++)
    {
        if(game[0][j] == game[1][j] && validPos[game[2][j] - '0'])
        {
            move = game[2][j] - '0';
            if(game[0][j] == computer)
                return move;
        }

        if(game[1][j] == game[2][j] && validPos[game[0][j] - '0'])
        {
            move = game[0][j] - '0';
            if(game[1][j] == computer)
                return move;
        }

        if(game[0][j] == game[2][j] && validPos[game[1][j] - '0'])
        {
            move = game[1][j] - '0';
            if(game[0][j] == computer)
                return move;
        }
    }

    // checks for the winning move along the right diagonal.
    // preferance is given to winning move for computer.
    if(game[0][0] == game[1][1] && validPos[game[2][2] - '0'])
    {
        move = game[2][2] - '0';
        if(game[0][0] == computer)
            return move;
    }

    if(game[0][0] == game[2][2] && validPos[game[1][1] - '0'])
    {
        move = game[1][1] - '0';
        if(game[0][0] == computer)
            return move;
    }

    if(game[1][1] == game[2][2] && validPos[game[0][0] - '0'])
    {
        move = game[0][0] - '0';
        if(game[1][1] == computer)
            return move;
    }

    // checks for the winning move along the left diagonal.
    // preferance is given to winning move for computer.
    if(game[0][2] == game[1][1] && validPos[game[2][0] - '0'])
    {
        move = game[2][0] - '0';
        if(game[0][2] == computer)
            return move;
    }

    if(game[0][2] == game[2][0] && validPos[game[1][1] - '0'])
    {
        move = game[1][1] - '0';
        if(game[0][2] == computer)
            return move;
    }

    if(game[1][1] == game[2][0] && validPos[game[0][2] - '0'])
    {
        move = game[0][2] - '0';
        if(game[1][1] == computer)
            return move;
    }

    // if there was any such move in which either the player or
    // the computer would win, then return it.
    if(move != 0)
        return move;

    // if there was no such move, then select any of the corners
    // or the center randomly.
    switch(rand() % 4)
    {

        case 1 : if(validPos[7] && validPos[3])
                    return 7;

        case 2 : if(validPos[3] && validPos[7])
                    return 3;

        case 3 : if(validPos[1] && validPos[9])
                    return 1;

        case 4 : if(validPos[5])
                    return 5;

        default : return pos;
    }
}

P.S I've not included the code snippet for functions disply(), isComplete() and gameStatus() as I think these are simple, and there may not be much to review there

Answer 1

Here are a number of things that may help you improve your code.

Match initializaton with declaration

The variable game is currently declared and initialized like this:

char game[3][3] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};

But those don't really match. I'd recommend instead to have them match by changing that line to this:

char game[3][3] = {{'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9'}};

Eliminate global variables where practical

Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error. Eliminating global variables where practical is always a good idea, whether programming for desktop machines or for embedded systems. In this code, I would recommend that all of the game state variables, including the board, the current turn and the tokens used could all be wrapped inside one structure and then a pointer to that structure could be passed to each of the relevant functions. This makes it much easier to understand and much easier to maintain.

Eliminate redundant variables

The validPos array is not really needed given that the game array is passed to each function. A position is "valid" in the sense of this game if it's not already occupied, so instead of this:

if(game[i][0] == game[i][1] && validPos[game[i][2] - '0'])

You could write this:

if(game[i][0] == game[i][1] && isdigit(game[i][2]))

Don't reseed the random number generator more than once

The program currently calls srand before every turn. This is really neither necessary nor advisable. Instead, just call it once when the program begins and then continue to use rand() to get random numbers.

Understand how rand() works

The rand() function returns a number between 0 and RAND_MAX. Just for simplicity, let's say that RAND_MAX is 15 (it's actually much larger than that). A random number generator (or psuedo-random number generator) should generate each possible number with equal frequency, so we'd have 0 occur one sixteenth of the time, 1 occur one sixteenth of the time, etc. So far so good, but let's see what happens when we use a construct like this:

pos = rand() % 10;

Here is the probability table:

rand    pos     probability
0       0           1/16
1       1           1/16
2       2           1/16
3       3           1/16
4       4           1/16
5       5           1/16
6       6           1/16
7       7           1/16
8       8           1/16
9       9           1/16
10      0           1/16
11      1           1/16
12      2           1/16
13      3           1/16
14      4           1/16
15      5           1/16

The resulting probability for pos, then is this:

pos     probability
0           2/16
1           2/16
2           2/16
3           2/16
4           2/16
5           2/16
6           1/16
7           1/16
8           1/16
9           1/16

Because 10 doesn't divide evenly into RAND_MAX, the numbers at the lower end of the range are statistically more likely to be selected than those at the end of the range. For a simple game like this, it may not matter much, but it's an important concept to understand. To make it so that you have an even distribution, you could do this:

int pos = rand() / (RAND_MAX / N + 1);

Where N is defined as the upper limit of your range (10 in this case).

Separate input, output and calculation

To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of your program (which does not depend on the underlying OS). The prompts for the user and the getting and validation of the user's move could be separated from the main game logic, maintaining a clean separation between I/O and game logic.

Use better naming

The function isComplete is not bad because it makes it obvious that a value of true returned would mean that the game is complete. (Although I might prefer isFull to distinguish between that state and the state in which one player has won.) The gameStatus function, however, does not have such an easy interpretation. So instead, I'd probably name that function isWon.

Don't use recursion when not needed

The recursive calls to startGame are not really needed. In this case, there's no great harm, but in cases in which there were many more calls, it could blow up the stack and cause your program to crash. A better way to do that is to isolate just the game logic so that instead of startGame we could name the function playGame and it might look like this:

bool playGame (void) {
    char game[3][3] = {{'1', '2', '3'}, {'4', '5', '6'}, {'7', '8', '9'}};
    bool humanTurn = true;
    while (!isWon(game) && !isFull(game))
    {
        display(game);
        if (humanTurn) {
            humanMove(game);
        } else {
            computerMove(game);
        }
        humanTurn = !humanTurn; 
    }
    return showResults(game, !humanTurn);
}

Now it's really clear how the game is structured, and the basic game logic could be used, unchanged, if this application were text-based or GUI-based.

Check return values for errors

The call of scanf can fail. You must check the return values to make it hasn't or your program may crash (or worse) when given malformed input. Rigorous error handling is the difference between mostly working versus bug-free software. You should strive for the latter.

Simplify your code

One way to restructure the overly long calcMove() function would be to calculate a score for each possible move and then pick the highest value. For example, you could have a 3 by 3 grid of integers that are initialized to 0. Then add points for each of the conditions you have already identified in the comments:

1. the move is valid and would win
2. the move is valid and would block a user win
3. the move is valid and is a corner or the middle
4. the move is valid and is an edge

Naturally, the first category is more valuable than the last, so you'd weight the points appropriately.