7
\$\begingroup\$

I made a simple connect four terminal game in C and was wondering if I could get feedback on it. I mainly want to know where pointers could have optimised my code, and if there are any major redundancies. ( I know that the is_occupied function is one, but I added that for readability) Here is the code:

/*  simple connect 4 game   */
/*  current version: 1.0    */

// inclusions 
#include <stdio.h>
#include <stdbool.h> 
#include <stdlib.h>
#include <windows.h>
#include <time.h>

// definitions 
#define LEN_X 7                                 // width of a normal board ( I think )
#define LEN_Y 6                                 // length of a normal board ( I think )


// global variables 
char board_red[LEN_Y][LEN_X];                   // the board where the game will be played on 

bool particle_exists_red[LEN_Y][LEN_X];         // a boolean board that holds the slots where red has a piece 
bool particle_exists_yellow[LEN_Y][LEN_X];      // a boolean board that holds the slots where yellow has a piece
bool particle_exists[LEN_Y][LEN_X];             // a boolean board that holds the slots where a piece exists in general
bool temp_particle_exists[LEN_Y][LEN_X];        // a boolean board that holds the value of each slot, for the temp print function
bool temp_particle_exists_red[LEN_Y][LEN_X];    
bool temp_particle_exists_yellow[LEN_Y][LEN_X]; 

int red_wins = 0; 
int yellow_wins = 0; 

bool win = false; 

// protoypes 
void particle_move_red(int x);                  // function in charge of moving the red pieces(particles)
void particle_move_yellow(int x);               // function in charge of moving the yellow pieces(particles)
void print_grid(void);                          // function that prints the finished grid
bool is_occupied(int x, int y);                 // functino used to see if a slot is occupied 
void print_temp_grid(void);                     // function that prints the temporary grid 
bool win_check(void);                           // finction that checks if winconditons are met
void yellow_plays(void);                        // lets the robot make a move 
void game_loop(void);                           // main game loop
void clear_grid(void);                          // function that clears the grid for subsequent games 

// main 
int main(void)
{
    srand(time(NULL)); 

    char keep_playing = 'y'; 
    do
    {
        printf("Do you want to play? (y/n): ");
        scanf(" %c", &keep_playing); 
        if(keep_playing == 'y')
        {
            game_loop(); 
        }
        else 
        {
            printf("You scored:      %d  points\nComputer scored: %d  points\n", red_wins, yellow_wins); 
        }
    } while (keep_playing == 'y');
    
    return 0;
}



// function that moves the red particle 
void particle_move_red(int x)
{
    int move_y = LEN_Y; 
    int i; 
    for(i = LEN_Y; i > 1; i--)
    {
        if(!is_occupied(x,move_y-1))
        {
            move_y--; 
            temp_particle_exists_red[x][move_y] = true; 
            print_temp_grid();
            temp_particle_exists_red[x][move_y] = false; 
            Sleep(60);
            system("cls"); 
        }
    }
    particle_exists[x][move_y] = true; 
    particle_exists_red[x][move_y] = true;
    win_check();
}

// function that moves the yellow particle 
void particle_move_yellow(int x)
{
    int move_y = LEN_Y; 
    int i; 
    for(i = LEN_Y; i > 1; i--)
    {
        if(!is_occupied(x,move_y-1))
        {
            move_y--; 
            temp_particle_exists_yellow[x][move_y] = true; 
            print_temp_grid();
            temp_particle_exists_yellow[x][move_y] = false; 
            Sleep(60);
            system("cls"); 
           
        }
    }
    particle_exists[x][move_y] = true; 
    particle_exists_yellow[x][move_y] = true;
    win_check();
}


// function that checks if a slot is occupied 
bool is_occupied(int x, int y)
{
    return particle_exists[x][y] == true;
}


// function that prints the falling pieces 
void print_temp_grid()
{
    int i, j; 
    printf("\n");
    printf("  |1|2|3|4|5|6|7|\n");
    for(i = LEN_Y; i > 0; i--)
    { 
        printf("  |"); 
        for(j = 1; j <= LEN_X; j++)
        {
            if(temp_particle_exists_red[j][i] == true || particle_exists_red[j][i] == true)
                printf("O|"); 
            else if(temp_particle_exists_yellow[j][i] == true || particle_exists_yellow[j][i] == true) 
                printf("X|"); 
            else
                printf(" |"); 
        }
        printf("\n"); 
    }
}

// function that checks for a win 
bool win_check(void)
{   
    int i, j; 
    for(i = LEN_Y; i > 0; i--)
    {
        for(j = 1; j <= LEN_X; j++)
        {
            // check for red win y axis 
            if (particle_exists_red[j][i] == true) 
                if (particle_exists_red[j][i+1] == true) 
                    if (particle_exists_red[j][i+2] == true)
                        if (particle_exists_red[j][i+3] == true)
                        {
                            printf("Red wins!\n");
                            red_wins++;
                            win = true; 
                            return true; 
                        }
                            

            // check for red win x axis
            if (particle_exists_red[j][i] == true) 
                if (particle_exists_red[j+1][i] == true) 
                    if (particle_exists_red[j+2][i] == true)
                        if (particle_exists_red[j+3][i] == true)
                        {
                            printf("Red wins!\n");
                            red_wins++;
                            win = true;
                            return true;
                        }

            // check for red win oblique from top y 
            if (particle_exists_red[j][i] == true) 
                if (particle_exists_red[j-1][i-1] == true) 
                    if (particle_exists_red[j-2][i-2] == true)
                        if (particle_exists_red[j-3][-3] == true)
                        {
                            printf("Red wins!\n");
                            red_wins++;
                            win = true; 
                            return true;
                        } 

            // check for red win oblique from bottom y 
            if (particle_exists_red[j][i] == true) 
                if (particle_exists_red[j+1][i+1] == true) 
                    if (particle_exists_red[j+2][i+2] == true)
                        if (particle_exists_red[j+3][+3] == true)
                        {
                            printf("Red wins!\n");
                            red_wins++;
                            win = true;
                            return true; 
                        } 
            // check for yellow win y axis 
            if (particle_exists_yellow[j][i] == true) 
                if (particle_exists_yellow[j][i+1] == true) 
                    if (particle_exists_yellow[j][i+2] == true)
                        if (particle_exists_yellow[j][i+3] == true)
                        {
                            printf("yellow wins!\n");
                            yellow_wins++;
                            win = true;
                            return true; 
                        }
                            

            // check for yellow win x axis
            if (particle_exists_yellow[j][i] == true) 
                if (particle_exists_yellow[j+1][i] == true) 
                    if (particle_exists_yellow[j+2][i] == true)
                        if (particle_exists_yellow[j+3][i] == true)
                        {
                            printf("yellow wins!\n");
                            yellow_wins++;
                            win = true;
                            return true;
                        }

            // check for yellow win oblique from top y 
            if (particle_exists_yellow[j][i] == true) 
                if (particle_exists_yellow[j-1][i-1] == true) 
                    if (particle_exists_yellow[j-2][i-2] == true)
                        if (particle_exists_yellow[j-3][-3] == true)
                        {
                            printf("yellow wins!\n");
                            yellow_wins++;
                            win = true; 
                            return true;
                        } 

            // check for yellow win oblique from bottom y 
            if (particle_exists_yellow[j][i] == true) 
                if (particle_exists_yellow[j+1][i+1] == true) 
                    if (particle_exists_yellow[j+2][i+2] == true)
                        if (particle_exists_yellow[j+3][+3] == true)
                        {
                            printf("yellow wins!\n");
                            yellow_wins++;
                            win = true; 
                            return true; 
                        } 
        }
    }
}

void yellow_plays(void)
{
    int randslot = rand() % 6 +1;

    particle_move_yellow(randslot); 

}


// main loop that plays until a win is reached 
void game_loop(void)
{
    int slot;
    win = false;
    clear_grid(); 
    while(!win)
    {   
        print_temp_grid(); 
        printf("Choose a slot to insert piece into(1 to 7): "); 
        scanf("%d", &slot);
        if (slot > 7 || slot < 1)
            printf("value is outside domain\n"); 
        else
        {
            system("cls"); 
            particle_move_red(slot); 
            if (!win)
                yellow_plays();
        }
         
        
    }
    
}

void clear_grid(void)
{
    int i, j;
    for(i = LEN_Y; i > 0; i--)
    {
        for(j = 1; j <= LEN_X; j++)
        {
            particle_exists[j][i] = false; 
            particle_exists_red[j][i] = false; 
            particle_exists_yellow[j][i] = false; 

        }
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Nice game. Don't forget to compile with -Wall \$\endgroup\$
    – ggorlen
    May 12, 2023 at 17:02
  • \$\begingroup\$ Didn’t know that that was a thing, thanks for the tip! \$\endgroup\$
    – Willem
    May 12, 2023 at 17:29

2 Answers 2

4
\$\begingroup\$

General Observations

If you are a true beginner this code is pretty good because it uses functions. In particular, main() calling game_loop() is a good design. Function names and variable names are clear and make the code easy to read.

The code is not portable because the code calls system("cls");, the #include <windows.h> could be within #ifdef WIN32, windows.h is not available on Linux systems.

I mainly want to know where pointers could have optimised my code

That depends, if you are using the optimizing feature of the compiler than it is unlikely that pointers would improve the performance of the code. If you are not using the optimizer than pointers might have improved the performance, but that isn't clear just from examining the code.

It is important to note that clear readable code is more important than optimized code, and that optimizing compilers really do a great job of making fast small code when they are used.

... if there are any major redundancies.

There are other redundancies from that you mentioned, the functions void particle_move_red(int x) and void particle_move_yellow(int x) very clearly redundant and if more arguments were passed in they could be one function. Another way to reduce the redundancies if the game board was only one data structure rather than multiple data structures. This could be done if the board was made of enums representing empty, red, or yellow rather than boolean value of true and false. The use of enums might simplify the function void print_temp_grid() as well. The function bool win_check(void) also contains redundancies that could be reduced, each logic block in win_check() could be a function call.

As the user ggorlen there are warnings when the code is compiled with higher levels of error checking:

Line 45: warning C4244: 'function': conversion from 'time_t' to 'unsigned int', possible loss of data
Line 248: warning C4715: 'win_check': not all control paths return a value

The second warning on line 248 is an indication that are possible logic errors or bugs in the code in the function win_check(). It is very interesting that there is no return false; in the function win_check(). The first warning might be fixed with a cast.

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.

Code Organization

Function prototypes are very useful in large programs that contain multiple source files, and that in case they will be in header files. In a single file program like this it is better to put the main() function at the bottom of the file and all the functions that get used in the proper order above main(). Keep in mind that every line of code written is another line of code where a bug can crawl into the code.

Useful Comments

Comments are generally used to indicate design decisions, other than that code should be self documenting with variable and function names that are clear. These are the 4 useful comments I see:

/*  simple connect 4 game   */
/*  current version: 1.0    */
#define LEN_X 7                                 // width of a normal board ( I think )
#define LEN_Y 6                                 // length of a normal board ( I think )

If the definitions were BOARD_LENGTH and BOARD_WIDTH the second set of comments would not be necessary.

Prefer Braces { and } Around Single Statements in if or loops

Some programmers consider this a style issue, but it makes it much easier to read and maintain the code if each in an if, else or loop block is embedded within braces. Extending the functionality of these statements can be problematic when the braces are not used. For a more in depth discussion of this see the first 2 answers on this Stack Overflow question. As one of the answers points out this is true in all C like languages (C, C++, C#, JavaScript, Java, etc.). I have worked at multiple companies where this was required in the coding standard and flagged during code reviews.

The code in win_check() is very difficult to maintain, it would be easier if the code looked like this:

            if (particle_exists_red[j][i] == true)
            {
                if (particle_exists_red[j][i + 1] == true)
                {
                    if (particle_exists_red[j][i + 2] == true)
                    {
                        if (particle_exists_red[j][i + 3] == true)
                        {
                            printf("Red wins!\n");
                            red_wins++;
                            win = true;
                            return true;
                        }
                    }
                }
            }
\$\endgroup\$
1
  • \$\begingroup\$ very insightful, thank you for your tips and time! \$\endgroup\$
    – Willem
    May 13, 2023 at 20:09
4
\$\begingroup\$

Add guards:

At the top of the file, for the non-standard function sleep() /Sleep():

#ifdef _WIN32
    #include <Windows.h>
#else
    #include <unistd.h>
#endif

With macros, you can determine whether you are using a Windows or a Unix-like system and call the respective function depending on your current platform:

static inline void clear_screen (void)
{
    #if defined(__linux__) || defined(__unix__) || defined(__APPLE__)
        system ("clear");
    #endif

    #if defined(_WIN32) 
        system ("cls");
    #endif
} 

Or a macro:

#if defined(__linux__) || defined(__unix__) || defined(__APPLE__) 
    #define CLEAR_SCREEN() system ("clear") 
#else
    #define CLEAR_SCREEN() system ("cls")
#endif

The same could be done about Sleep():

static inline void snooze (unsigned int time) 
{
    #ifdef _WIN32
        Sleep (time);
    #else
        usleep (time) */
    #endif

    /* Or you could define a macro. */
}

Nota bene that POSIX.1-2001 declares this function obsolete; use nanosleep(2) instead. POSIX.1-2008 removed the specification of usleep().


Check the return of system/library calls:

scanf() returns the number of conversions successfully performed. We ought to check its return to ensure that we did indeed read an int:

// scanf("%d", &slot);

if (scanf ("%d", &slot) != 1) {
    complain();
}

Is the above solution foolproof? No.

See: How to properly read an int? and A beginner's guide away from scanf().


Reduce scope:

Since C99, variables can be declared in the for loop's control expression.

#if 0
    int i; 
        for(i = LEN_Y; i > 1; i--)
#else 
    for (int i = LEN_Y; i > 1; i--) {}
#endif

Note that i is local to the for loop block only, and will cease to exist once it goes out of scope.


Simplify code:

/*
if (particle_exists_red[j][i] == true) 
    if (particle_exists_red[j-1][i-1] == true) 
        if (particle_exists_red[j-2][i-2] == true)
            if (particle_exists_red[j-3][-3] == true)
            {
                printf("Red wins!\n");
                red_wins++;
                win = true; 
                return true;
            } 
*/
 
if (particle_exists_red[j][i] 
    && particle_exists_red[j-1][i-1]
    && particle_exists_red[j-2][i-2] 
    && particle_exists_red[j-3][-3]) {
    printf("Red wins!\n");
    red_wins++;
    return win = true;
}

// Or:

bool status = particle_exists_red[j][i] 
              && particle_exists_red[j-1][i-1]
              && particle_exists_red[j-2][i-2] 
              && particle_exists_red[j-3][-3];

if (status) { 
    printf("Red wins!\n");
    red_wins++;
    return win = true;
}

Minor:

  • Use unsigned types for cardinalities, ordinal numbers and sizes. (For example, array indexes can never be negative.)
  • Prefer return EXIT_SUCCESS to return 0.
  • Prefer puts() to printf() when the latter's parsing capabilities are not required. Yes, the compiler may indeed replace the call with puts() by itself, but don't let the compiler do what you can trivially do yourself with greater code clarity.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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