First C program - Tic Tac Toe

Question

Hi I am a beginner and l would love to hear your opinion on my first C project, how can it be improved in any way, for example dynamic memory allocation isn't used in the program because I didn't quite understand where to allocate it, also I couldn't figure how to solve the problem that if input isn't an integer the program goes into an endless loop

Thanks for anyone who helps

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

void print();
void change(int num, int a);
void restart();
bool draw();
bool win();
bool check(char a[]);
bool CheckInput(int num);

char a[9] = {49,50,51,52,53,54,55,56,57};

void main(){ 
int b[9] = {1,2,1,2,1,2,1,2,1}; 
int num;
restart();
for(int i = 0; i < 9; i++)
{
    system("cls");
    print();
    printf("player %d enter a number option ",b[i]);
    scanf("%d",&num);
    change(num,b[i]);
    if(i == 8)
    {
    system("cls");
    print();
    }
    if(win() == true)
    {
        print();
        printf("payer %d wins",b[i]);
        return;
    }
}
printf("its a draw"); 
return;
} 

bool CheckInput(int num)
{
char (*str)[9] = &a;
if(num > 9 || num < 1)
return false;
else if(*(*str+(num-1)) == 79 || *(*str+(num-1)) == 88)
return false;
return true;
}

void change(int num, int p){
char (*str)[9] = &a;
int n = 0;
if(CheckInput(num) == true)
{
    if(p == 1)
    *(*str+(num-1)) = 88;
    else
    *(*str+(num-1)) = 79;
    return;
}
else
printf("\nincorrect enter a new number ");
scanf("%d",&n);
change(n,p);
}

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

void restart()
{
    char (*str)[9] = &a;
    static int j = 49;
    for(int i = 0; i < 9; i++)
    {
        *(*str + i)  = j;
        j++;
    }
    print();
}

bool win(){
char (*str)[9] = &a;
char b[3];
char c[3];
for(int i = 0; i < 3; i++)
{
    for(int j = 0; j < 3; j++)
    {
        b[j] = *(*str + i * 3 + j);//columns
        c[j] =  *(*str + j * 3 + i);//rows
    }
    if(check(b) == true || check(c) == true)
    return true;
}
int j =0;
for(int i =0; i<3; i++)
{
    b[i] = *(*str + i * 3 + j);//left to right across 
    j++;
}
int k = 2;
for(int i =0; i < 3; i++)
{
    c[i] = *(*str + i*3 + k);//right  to left across 
    k--;
    
}
if(check(b) == true || check(c) == true)
return true;
return false;
}

bool check(char a[3]){
int count = 0;
int count2 = 0;
    for(int i =0; i<3; i++)
    {
        if(a[i] == 88)
        count++;

        else if(a[i] == 79)
        count2++;
    }
    if(count != 3 && count2 !=3)
    return false;
    return true; 
}

Answer 1

Welcome to Code Review! Here's my two cents:

Magic chars

Across the entire program, you use the ASCII values of chars instead of the chars themselves. This makes the code hard to read, as the reader sees a bunch of numbers at first an then has to infer what they mean, i.e. that they are actually chars.

This means, that e.g. char a[9] = {49,50,51,52,53,54,55,56,57};
should be written as char a[9] = {'1','2','3','4','5','6','7','8','9'};
Similarly, 88 -> 'X' and 79 -> 'O'.

In the end, this is just a different, more legible way of writing the char codes. C will happily calculate '9'-'7'.

Inconsistent naming

CheckInput should be renamed to either check_input or checkInput. C doesn't have one definitive convention, but non convention I know of uses PascalCase for functions. Also, it's inconsistent with the rest of your functions.

Inexpressive names

A lot of the var names are one char wide (a, b, i, j, ...), making it hard to understand what they represent. It's fine for indexes used inside for-loops, but every other var should have a more descriptive name. For example, a should probably be named something like cell_values.

Same goes with the functions: If print() prints the board, why not name the function print_board()? Reconsider the other function names as well.

Indentation and formatting

Consider using an editor that has an auto-formatting feature (VSCode/VSCodium, VS, Eclipse, SublimeText, ...). As is, the formatting is very inconsistent, making the code hard to read.

Fun pointer things

// somewhere in change()

char (*str)[9] = &a;
// ...
*(*str+(num-1)) = 88;

Ummm... This is something to do when you want to actively confuse whoever is trying to read your code.

Since you've defined a outside of any function at the very top, the rest you your program can just access it as normal (read: without storing the adress in another var like str and dereferencing).
Hence, you should delete the first line and change the other one to a[num-1] = 88;. Yes, in the end the compiler will interpret this as *(a+(num-1)) = 88;, but indexing into an array using [] is easier to read than pointers. Similarly, in win(), *(*str + i * 3 + j) is the same as a[i * 3 + j].

Comparing to boolean

if (win() == true) is the same as if (win()) and for the sake of reducing visual clutter, it should be written like this. Coming back to a previous point, win() should have it's name chosen so that the entire line can be read as a sentence of some sort. Maybe if (game_is_won())? Same goes for for the use of check() and CheckInput().

Printing the board

IMO in this case, printing the board with 9 printfs would also be fine. Also, currently the first and last row of the board are printed in two lines, while the second row is third lines tall.

static

In restart(), you have a line static int j = 49;. Making the variable static doesn't make much sense, since you neither need to limit it's scope across multiple files (there is only one file) nor re-use the value when the function gets called again (which it doesn't).

if without curly braces

Even if the if only has one line, use curly braces. I find it easier to read and it prevents bugs if you want to add or remove lines from the if-statement.

Tip regarding newlines

I find it easier to keep track of the newlines if I put them at the end of a print statement. Also, the win/draw messages should have a trailing newline, since it messes with any text that might follow (such as a command prompt).

Hint regarding system("cls")

This isn't portable. Also, the use of system is highly discouraged (apparently).

As you're a beginner, don't worry about any of this. Still, remember that some things might only work on windows and some on linux.

Typo

printf("payer %s wins", b[i]);. This should be a player.

Answer 2

Some things to add after @mindoverflow review.

Enable all warnings

Save time, enable all warnings.

warning: return type of 'main' is not 'int' [-Wmain]
   25 | void main(){

In function 'restart':
warning: conversion from 'int' to 'char' may change value [-Wconversion]
  101 |         *(*str + i)  = j;


warning: argument 1 of type 'char[3]' with mismatched bound [-Warray-parameter=]
  139 | bool check(char a[3]){
      |            ~~~~~^~~~
note: previously declared as 'char[]'
   20 | bool check(char a[]);
      |  

Error checking

Do not trust user to enter only numeric text

// scanf("%d",&num);
if (scanf("%d",&num) != 1) {
  Handle_input_error(); // TBD code
}

Simplify

//if(count != 3 && count2 != 3)
//return false;
//return true;

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

Missing comments

Add some salient comments.

---

how to solve the problem that if input isn't an integer the program goes into an endless loop

Avoid scanf(). Its error handling is very cumbersome.

Use fgets() to read a line of user input into a string, then process the string with strtol(), sscanf(), etc.

#define NUM_MIN 1
#define NUM_MAX 9
#define LINE_SIZE 80   // Expected max line size
char buf[2*LINE_SIZE]; // Use 2x, be generous

do {
  printf("player %d enter a number option ", b[i]);
  if (fgets(buf, sizeof buf, stdin) == NULL) {
    Handle_No_Input(); // Perhaps exit
  }
} while (sscanf(buf, "%d", &num) != 1 || num < NUM_MIN || num > NUM_MAX);