Text-based Tic Tac Toe in C

Question

I have programmed Tic Tac Toe in C. How can I improve the code? I have been told that system("cls") is not secure, but I don't know any other way of clearing the screen.

main.c:

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

#define BOOL short int
#define TRUE 1
#define FALSE 0

#define NUMBER_OF_ROWS 3
#define NUMBER_OF_COLUMNS 3

void setup_grid(char (*grid)[NUMBER_OF_COLUMNS]);
void display_grid(char (*grid)[NUMBER_OF_COLUMNS]);
void add_piece(char (*grid)[NUMBER_OF_COLUMNS], char, char);

BOOL space_is_empty(char (*grid)[NUMBER_OF_COLUMNS], char);
BOOL three_in_a_row(char (*grid)[NUMBER_OF_COLUMNS], char);

int main()
{
    char grid[NUMBER_OF_ROWS][NUMBER_OF_COLUMNS];
    setup_grid(grid);

    char player = '1';
    char player_piece = 'O';

    char space;
    int space_int;

    int piece_count = 0;

    BOOL program_is_running = TRUE;
    BOOL input_loop;

    while(program_is_running)
    {
        system("cls");
        display_grid(grid);
        
        do
        {
            printf("Player %c: ", player);
            space = getchar();
            getchar(); // For the \n char.
            printf("\n");

            space_int = space - '0';

            if(space_int < 1 || space_int > 9)
            {
                printf("Please enter an integer from 1-9\n");
                input_loop = TRUE;
            }
            else if(space_is_empty(grid, space))
            {
                add_piece(grid, player_piece, space);
                piece_count++;
                input_loop = FALSE;
            }
            else
            {
                printf("That space is already taken.\n");
                input_loop = TRUE;    
            }
        }
        while(input_loop);

        if(three_in_a_row(grid, player_piece))
        {
            system("cls");
            display_grid(grid);
            printf("Player %c won!", player);
            program_is_running = FALSE;
        }
        else if(piece_count >= 9)
        {
            system("cls");
            display_grid(grid);
            printf("It is a draw.\n");
            program_is_running = FALSE;
        }
        else if(player == '1')
        {
            player = '2';
            player_piece = 'X';
        }
        else
        {
            player = '1';
            player_piece = 'O';    
        }
    }

    getchar();    

    return 0;
}

void setup_grid(char (*grid)[NUMBER_OF_COLUMNS])
{
    int count = 1;

    for(int y = 0; y < NUMBER_OF_ROWS; y++)
    {
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++, count++)
        {    
            grid[y][x] = count + '0';
        }
    }
}

void display_grid(char (*grid)[NUMBER_OF_COLUMNS])
{
    printf("\n");
    for(int y = 0; y < NUMBER_OF_ROWS; y++)
    {
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
        {
            printf(" %c ", grid[y][x]);
        }

        printf("\n");
    }
    printf("\n");
}

void add_piece(char (*grid)[NUMBER_OF_COLUMNS], char player, char space)
{
    for(int y = 0; y < NUMBER_OF_ROWS; y++)
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
            if(grid[y][x] == space)
                grid[y][x] = player;
}

BOOL space_is_empty(char (*grid)[NUMBER_OF_COLUMNS], char space)
{
    for(int y = 0; y < NUMBER_OF_ROWS; y++)
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
            if(grid[y][x] == space)
                return TRUE;

    return FALSE;
}

BOOL three_in_a_row(char (*grid)[NUMBER_OF_COLUMNS], char player)
{
    // Horizontal check.
    
    for(int y = 0; y < NUMBER_OF_ROWS; y++)
    {
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
        {
            if(grid[y][x] == player)
            {
                if(x == 2)
                    return TRUE;
            }
            else
                break;
        }
    }

    // Vertical check.
    
    for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
    {
        for(int y = 0; y < NUMBER_OF_ROWS; y++)
        {
            if(grid[y][x] == player)
            {
                if(y == 2)
                    return TRUE;
            }
            else
                break;
        }
    }

    // Diagonal check.
    
    // Top left to bottom right.
    
    if(grid[0][0] == player && grid[1][1] == player && grid[2][2] == player)
        return TRUE;

    // Bottom left to top right
    
    if(grid[2][0] == player && grid[1][1] == player && grid[0][2] == player)
        return TRUE;

    return FALSE;
}

Answer 1

I see some things that may help you improve your code.

Prefer stdbool.h to defining your own BOOL type

There is a header stdbool.h that has been part of the standard since C99. It's generally better to rely on standard types rather than creating your own duplicate. In this case, use bool rather than your BOOL.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    printf("\x1b[2J");
}

Think of the user

Rather than referring to "Player 1" and "Player 2" wouldn't it be easier for the player to refer instead to "Player O" and "Player X"? This has the additional advantage of simplifying the program as well, since it's no longer necessary to have two variables to track.

Use a typedef to simplify declarations

The code has quite a number of references to a grid type:

void display_grid(char (*grid)[NUMBER_OF_COLUMNS]);

This is peculiar in that it seems to make the implicit claim that only the NUMBER_OF_COLUMNS is important because NUMBER_OF_ROWS is not mentioned. This is a good use of a typedef:

typedef char GridType[NUMBER_OF_ROWS][NUMBER_OF_COLUMNS];
void display_grid(GridType grid);

This makes things a bit neater and easier to read and understand.

Simplify your code

The code currently contains this function

BOOL space_is_empty(char (*grid)[NUMBER_OF_COLUMNS], char space)
{
    for(int y = 0; y < NUMBER_OF_ROWS; y++)
        for(int x = 0; x < NUMBER_OF_COLUMNS; x++)
            if(grid[y][x] == space)
                return TRUE;

    return FALSE;
}

There's really no need to cycle through all of the values. It could just as well compute the location of the square and check it directly. It doesn't matter much with only a 3x3 grid, but it's good to begin to develop the habit of using better algorithms. I'd suggest adding a function that translates from the number the user enters to a pointer to the corresponding square. It would make several functions much simpler and easier to understand.

Return something useful from functions

Consider that if your add_piece were to simply return a boolean value if the placement of the piece were successful, you could use that and eliminate the largely duplicated code in space_is_empty.

Consider making the code more generic

Right now, the code uses #defines for both the NUMBER_OF_ROWS and NUMBER_OF_COLUMNS which is good. However, the code will break if anything other than 3 is used because of the three_in_a_row code that uses hardcoded values rather than those values:

// Diagonal check.

// Top left to bottom right.

if(grid[0][0] == player && grid[1][1] == player && grid[2][2] == player)
    return true;

// Bottom left to top right

if(grid[2][0] == player && grid[1][1] == player && grid[0][2] == player)
    return true;

return false;

I would bet that with a little thought, you could devise code that would work correctly for any size square, which leads me to the next suggestion:

Consider non-square boards

If you want to support a non-square board, such as 4 x 5, then consider how that might alter the rules for checking for a win. Is there such thing as a diagonal win? If you don't wish to support non-square boards, then it would probably make sense either use a single dimension like this:

#define DIMENSION_OF_GRID 3

Or to define one in terms of the other:

#define NUMBER_OF_ROWS 3
// columns must always equal rows for a square board
#define NUMBER_OF_COLUMNS NUMBER_OF_ROWS