0
\$\begingroup\$

The code's structure has been changed to make the program more readable. I've also commented and split up the program into blocks for better readability as well. The recursion in the player and computer moves have been replaced with a loop instead. The user can now determine the first player. The algorithm to determine game end by a win has also been made much more concise by re-using code and using loops. User input is also now validated, throwing up an error for invalid inputs.

I still have not made a smarter computer, or used a minmax algorithm, and I am working on extending the program to allow user to make choices about players. However regarding the code at hand, are there any mistakes I am making in making certain choices, or any further optimisations I can make to improve structure, readability, efficiency, safety, etc?

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

//----------------------------------------------------------------------------------
// Structure Definition
//----------------------------------------------------------------------------------
struct game_data {
    int win;         // Either 0 or 1.
    int turns;       // Ranges from 0 to 9(game end).
    int turn;        // Either 0 or 1 where 0 is human player
    char grid[3][3]; // Grid
};

//------------------------------------------------------------------------------------
// Function Declarations
//------------------------------------------------------------------------------------
void Intro(void);                                   // Game intro
void GameSetup(struct game_data *game);             // Determines first player
void PlayerOneMove(struct game_data *game);         // Player one input
void RandomComputerMove(struct game_data *game);    // Computer move
void DrawUpdatedGrid(const struct game_data *game); // Draw updated grid
void GameEventWon(struct game_data *game);          // Checks for winners
void GameEventDrawn(struct game_data *game);        // Checks for draws
char ThreeInARow(char grid[][3]);                   // Checks for three-in-a-row

//------------------------------------------------------------------------------------
// Main Loop
//------------------------------------------------------------------------------------
int main(void) {
    // Initialises Random Number Generator
    srand((unsigned)time(0));

    // Initialising Game State Variables
    struct game_data game = {
        0, 
        0, 
        0, 
        {{' ', ' ', ' '}, 
         {' ', ' ', ' '}, 
         {' ', ' ', ' '}}
    };

    Intro();
    GameSetup(&game);

    while (game.win == 0) {

        if (game.turn == 0) {

            PlayerOneMove(&game);
            // RandomComputerMoveTwo(&game);

            game.turns++;
            game.turn = 1;
        }

        else {

            RandomComputerMove(&game);

            game.turns++;
            game.turn = 0;
        }

        DrawUpdatedGrid(&game);

        GameEventWon(&game);
        GameEventDrawn(&game);
    }

    return 0;
}

//------------------------------------------------------------------------------------
// Short Intro and Setup
//------------------------------------------------------------------------------------
void Intro(void) {

    printf("\nWelcome to NOUGHTS AND CROSSES\n\n");
    printf("The grid you will be playing on is 3x3 and your input will be "
           "determined by the co ordinates you put in, in the form 'row "
           "column'.\n\n");
    printf("For example an input of '1 1' will put a 'Z' on the first row "
           "on the "
           "first column. Like so:\n\n");

    printf("    1   2   3  \n"
           "  +---+---+---+\n"
           "1 | Z |   |   |\n"
           "  +---+---+---+\n"
           "2 |   |   |   |\n"
           "  +---+---+---|\n"
           "3 |   |   |   |\n"
           "  +---+---+---+\n"

           "\n");
}

void GameSetup(struct game_data *game) {
    char answer;

    // Allow user to determine first player
    printf("Would you like to play first? y/n \n");
    answer = getchar();

    if (answer == 'y') {
        game->turn = 0; 
        printf("\nYou have chosen to go first\n");
    } else if (answer == 'n') {
        game->turn = 1; 
        printf("\nYou have chosen to go second\n");
    }

    else {
        puts("Invalid input");
        exit(EXIT_FAILURE);
    }
}

//------------------------------------------------------------------------------------
// Player One Move
//------------------------------------------------------------------------------------
void PlayerOneMove(struct game_data *game) {
    int row, column;

    do {
        printf("You are 'Crosses'. Please input co-ordinates ...\n");
        if (scanf(" %d %d", &row, &column) != 2) {
            puts("Invalid input");
            exit(EXIT_FAILURE);
        }
        if (row < 1 || row > 3 || column < 1 || column > 3) {
            puts("Out of range input");
            exit(EXIT_FAILURE);
        }

    } while (game->grid[row - 1][column - 1] != ' ');
    game->grid[row - 1][column - 1] = 'X'; // Places player move
    printf("\nYour turn:\n\n");
}

//------------------------------------------------------------------------------------
// Computer Move
//------------------------------------------------------------------------------------
void RandomComputerMove(struct game_data *game) {
    int column;
    int row;

    do {
        row = rand() % 3;
        column = rand() % 3;
    } while (game->grid[row][column] != ' ');

    printf("\nComputer turn:\n\n");
    game->grid[row][column] = 'O'; // Places computer move
}

//------------------------------------------------------------------------------------
// Display Updated Grid
//------------------------------------------------------------------------------------
void DrawUpdatedGrid(const struct game_data *game) {

    printf(

        "+---+---+---+\n"
        "| %c | %c | %c |\n"
        "+---+---+---+\n"
        "| %c | %c | %c |\n"
        "----+---+---|\n"
        "| %c | %c | %c |\n"
        "+---+---+---+\n"
        "\n",

        game->grid[0][0], game->grid[0][1], game->grid[0][2], game->grid[1][0],
        game->grid[1][1], game->grid[1][2], game->grid[2][0], game->grid[2][1],
        game->grid[2][2]);
}

//------------------------------------------------------------------------------------
// Checks for Three In A Row
//------------------------------------------------------------------------------------
char ThreeInARow(char grid[][3]) {
    for (int i = 0; i < 3; i++) {

        // Rows
        if (grid[i][0] == grid[i][1] && grid[i][1] == grid[i][2]) {
            return grid[i][0];
        }

        // Columns
        else if (grid[0][i] == grid[1][i] && grid[1][i] == grid[2][i]) {
            return grid[0][i];
        }
    }

    // Diagonals
    if ((grid[0][0] == grid[1][1] && grid[1][1] == grid[2][2]) ||
        (grid[0][2] == grid[1][1] && grid[1][1] == grid[2][0])) {
        return grid[1][1];
    }

    return ' ';
}

//------------------------------------------------------------------------------------
// Check if Game Won
//------------------------------------------------------------------------------------
void GameEventWon(struct game_data *game) {
    switch (ThreeInARow(game->grid)) {
    case 'X':
        game->win = 1;
        printf("CROSSES has WON\n");
        break;
    case 'O':
        game->win = 1;
        printf("NOUGHTS has WON\n");
        break;
    }
}

//------------------------------------------------------------------------------------
// Check if Game Drawn
//------------------------------------------------------------------------------------
void GameEventDrawn(struct game_data *game) {

    if (game->turns == 9 && game->win == 0) {
        game->win = 1;
        printf("GAME is a DRAW\n");
    }
}
\$\endgroup\$
1
\$\begingroup\$

I really like your program because it is well structured and very readable.

Bug

There is a bug in ThreeInARow(). For

+---+---+---+
|   |   |   |
+---+---+---+
| X | X | X |
----+---+---|
| O | O |   |
+---+---+---+

it returns ' ' (space) and the game continues.

Declaration and Initialization

If you don't have to support ancient compilers I would recommend to declare variables where you initialize them, e.g. /* ... */ char answer = getchar(); instead of char answer; /* ... */ answer = getchar();.

Naming

board_state.win signals if the game has ended (also in case of a draw) and therefore should be renamed to something like finished or ended.

Personally, I like to use a verb in my function names because I think it's clearer what the function does (get, set, write, initialize, etc.).

Comments

Some comments state the obvious:

// Initialises Random Number Generator
srand((unsigned)time(0));

or repeat what is already expressed by the variable or function name:

//------------------------------------------------------------------------------------
// Player One Move
//------------------------------------------------------------------------------------
void PlayerOneMove(struct game_data *game) {

My advice: Prefer expressive code over comments, because over the lifetime of a program (with bugfixes, refactorings and new features) comments and code tend to get out of sync.

DRY

You could refactor

if (game.turn == 0) {
    PlayerOneMove(&game);
    game.turns++;
    game.turn = 1;
}
else {
    RandomComputerMove(&game);
    game.turns++;
    game.turn = 0;
}

to

if (game.turn == 0)
    PlayerOneMove(&game);
else
    RandomComputerMove(&game);
game.turns++;
game.turn = !game.turn;

Error handling

Currently you handle all errors locally by calling exit(EXIT_FAILURE);. That works well for this program and keeps the error handling code small.
For larger programs you might prefer returning error codes and handling the errors in the main loop because it gets harder to reason about a program with many functions that might call exit().

Usability

I like your example where 1 1 places a Z on the board. IMHO something like 1 2 would be a little bit more helpful because it shows that the order is <row> <column>.

Code Style

I don't see PascalCase (GameEventDrawn()) very often for function names in C. Usually snake_case or lowerCamelCase are used.

Last not least I want to repeat myself:
I really like how readable your program is.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.