2
\$\begingroup\$

I have already posted this program and got some good recommendations on how it can be improved but I would like to further know what can be improved for example:

  1. Should dynamic memory allocation be used and if yes, where should I allocate it ?
  2. How can I replace system("cls") in the program ?
  3. Should I get rid the global variable ?
  4. I got recommend to enable all warnings I couldn't figure out how to do that
#include<stdlib.h>
#include<stdio.h>
#include<stdbool.h>
#define NUM_MIN 1
#define NUM_MAX 9
#define LINE_SIZE 80   

void print_board();
int get_input(int player);
bool check_input(int user_input);
void change_board(int user_input, int player);
bool check_win(char arr_values[3]);
bool player_has_won();

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

int main(){ 
    int players_turn[9] = {1,2,1,2,1,2,1,2,1}; 
    int cell_num = 0; 
    for(int i = 0; i < 9; i++)
    {
        system("cls");
        print_board();
        cell_num = get_input(players_turn[i]);
        change_board(cell_num,players_turn[i]);
        if(i == 8)
        {
            system("cls");
            print_board();
        }
        if(player_has_won())
        {
            system("cls");
            print_board();
            printf("player %d wins \n",players_turn[i]);
            return 0;
        }
    }
    printf("its a draw\n"); 
    return 1;
} 

void print_board(){    
    printf("\n");
    int j = 0;
    for (int i = 0; i < 2; i++) 
    {
        if(i == 1)
        {
            j = 3;
        }
        printf("  %c  |  %c  |  %c\n",g_cell_values[j],g_cell_values[j+1],g_cell_values[j+2]);
        printf("_____|_____|_____\n");
        printf("     |     |\n");
    }
    j = 6;
        printf("  %c  |  %c  |  %c\n",g_cell_values[j],g_cell_values[j+1],g_cell_values[j+2]);
}

int get_input(int player){
    int num = 0;
    char buf[2*LINE_SIZE]; 
    do {
        printf("player %d enter a number option ", player);
        if(fgets(buf, sizeof buf, stdin) == NULL) {
            printf("error occurred\n");
            exit(EXIT_FAILURE);
        }
    } while (sscanf(buf, "%d", &num) != 1 || num < NUM_MIN || num > NUM_MAX);
    return num;
}

bool check_input(int user_input){
    if(g_cell_values[user_input-1] == 'O' || g_cell_values[user_input-1] == 'X') 
     {
        return false;
     }
    else
    {
        return true;
    }
}

void change_board(int user_input, int player){
    int new_input = 0; 
    if(check_input(user_input))
    {
        if(player == 1)
        {
           g_cell_values[user_input-1] = 'X'; 
        }
        else
        {
            g_cell_values[user_input-1] = 'O';            
        }
        return;
    }
    else
    {
        new_input = get_input(player);
        change_board(new_input,player);
    }
}

bool check_win(char arr_values[3]){
    int count = 0;
    int count2 = 0;
    for(int i =0; i<3; i++)
    {
        if(arr_values[i] == 'X') 
        {
            count++;
        }

        else if(arr_values[i] == 'O') 
        {
            count2++;
        }
    }
    if(count == 3 || count2 == 3)
    {
        return true;
    }
    else
    {
        return false; 
    }
}

bool player_has_won(){
    char cell_values[3];  
    char cell_values2[3];  
    for(int i = 0; i < 3; i++)
    {
        for(int j = 0; j < 3; j++)
        {
            cell_values[j] = g_cell_values[i * 3 + j];//rows
            cell_values2[j] =g_cell_values[j * 3 + i];//columns 
        }
        if(check_win(cell_values) || check_win(cell_values2))
        {
            return true;
        }
    }
    int j =0;
    for(int i =0; i<3; i++)
    {
        cell_values[i] = g_cell_values[i * 3 + j];//left to right across 
        j++;
    }
    int k = 2;
    for(int i =0; i < 3; i++)
    {
        cell_values2[i] = g_cell_values[i*3 + k];//right  to left across 
        k--;   
    }
    if(check_win(cell_values) || check_win(cell_values2))
    {
        return true;
    }
    else
    {
        return false;
    }

}






  
 
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

1. Dynamic memory allocation

There is no need to dynamically allocate memory for the application.

2. Clearing the screen

There is no portable way to clear the screen. The following works on most operating systems:

system("cls||clear");

I would recommend creating a separate function to handle this so that if you need to change it, you would only need to do so in one location:

void clear_screen() {
    system("clear||cls");
}

3. Global variables

I think that using global variables for this specific solution is fine. You could pass cell_values to the functions directly if you did not want to have one:

int get_input(char cell_values[9], int player);

int main() {
    char cell_values[9] = { '1','2','3','4','5','6','7','8','9' };
    int players_turn[9] = { 1,2,1,2,1,2,1,2,1 };

    int cell_num = get_input(cell_values, players_turn[i]);
}

4. Enabling warnings

This will really depend on what compiler you are using.

More notes

You should call check_input from get_input instead of recursively calling change_board:

int get_input(int player) {
    int num = 0;
    char buf[2 * LINE_SIZE];
    do {
        printf("player %d enter a number option ", player);
        if (fgets(buf, sizeof buf, stdin) == NULL) {
            printf("error occurred\n");
            exit(EXIT_FAILURE);
        }
    } while (sscanf(buf, "%d", &num) != 1 || check_input(num) == false);
    return num;
}

bool check_input(int user_input) {
    if (user_input < NUM_MIN || user_input > NUM_MAX)
        return false;
    char cell_value = g_cell_values[user_input - 1];
    return cell_value == 'O' || cell_value == 'X' ? false : true;
}

void change_board(int user_input, int player) {
    g_cell_values[user_input - 1] = player == 1 ? 'X' : 'O';
}

You do not need an if statement if you are simply going to return a bool. In the last part of check_win you could have simply used:

return count == 3 || count2 == 3;

The same applies to player_has_won:

return check_win(cell_values) || check_win(cell_values2);
\$\endgroup\$
2
  • \$\begingroup\$ I am using mingw gcc compiler if it helps, but do you think its a good idea to enable all warnings ? \$\endgroup\$ Oct 29, 2021 at 18:16
  • \$\begingroup\$ @BobTheCoder, I really don't know gcc well enough to help. \$\endgroup\$
    – jdt
    Oct 29, 2021 at 18:49
1
\$\begingroup\$

Enabling warnings

I mostly see and use -Wall -Wextra. Is it a good idea? Yes, I think so. More often than not, the warnings turn out to be inaccuracies that cause bugs. Be sure to read and understand them, as in some cases the warning is a false alarm. As an example, I've had gcc complain about gethostname being undefined while unistd.h was included and the program still worked fine.

I don't recommend using -Werror, especially while initially writing the program. gcc will then throw every warning as an error, so you won't be able to ignore some warnings that you know are fine when you want to test something quickly (e.g. unused functions).

\$\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.