4
\$\begingroup\$

I'm trying to learn programming and this Tic-Tac-Toe is one of my first projects. The user gets to start first as 'x' or second as 'o'.

Please give me any criticism you find on the code and give suggestions on where I can improve the structure and style.

#include <stdio.h>
#define MAX(x, y) (((x) > (y)) ? (x) : (y))
#define MIN(x, y) (((x) < (y)) ? (x) : (y))
/*the print function will be called after every input by the user*/
void print(char (*GRID)[7])
{
  /*length and breadth of the grid of a tic-tac-toe game is 3*3*/
  int length=7, breadth=7,i,j;
      printf("\nTic-Tac-Toe by Udhay Sankar\n\n");
  for(i=0;i<length;i++)
  {
    printf("       ");
   for(j=0;j<breadth;j++)
   {
    printf("%c",GRID[i][j]);
   }
  printf("\n");
  }
}
/*function to check the validity of user input*/
int valid(char (*GRID)[7],int TEMP1)
{
  if(((TEMP1)>0)&&((TEMP1)<10))
  {
      switch (TEMP1)
      {
        case 1:
              if (GRID[1][1]=='x'||GRID[1][1]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 2:
              if (GRID[1][3]=='x'||GRID[1][3]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 3:
              if (GRID[1][5]=='x'||GRID[1][5]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 4:
              if (GRID[3][1]=='x'||GRID[3][1]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 5:
              if (GRID[3][3]=='x'||GRID[3][3]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 6:
              if (GRID[3][5]=='x'||GRID[3][5]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 7:
              if (GRID[5][1]=='x'||GRID[5][1]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 8:
              if (GRID[5][3]=='x'||GRID[5][3]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
        case 9:
              if (GRID[5][5]=='x'||GRID[5][5]=='o')
              {
                printf("\nPosition already occupied!!!\n");
                return 0;
              }
              else
                return 1;
        break;
      }
  }
  else
  {
    return 0;
  }
}
/*function to get input from user*/
void get_input(char (*GRID)[7],int turn)
{
  int TEMP2;
  if(1==turn)
  {
    printf("\nConsider the 3*3 grid with 9 positions\n\nIf position is row 2 column 3, Enter 6 as input\n\nEnter your position for 'x': ");
  }
  else
  {
    printf("\nConsider the 3*3 grid with 9 positions\n\nIf position is row 2 column 3, Enter 6 as input\n\nEnter your position for 'o': ");
  }
  scanf("%d",&TEMP2);
  while((valid(GRID,TEMP2))!=1)
  {
    //if position is already occupied
    printf("\nInvalid Input! Enter a valid position\n");
    if(1==turn)
    {
      printf("\nConsider the 3*3 grid with 9 positions\n\nIf position is row 2 column 3, Enter 6 as input\n\nEnter your position for 'x': ");
    }
    else
    {
      printf("\nConsider the 3*3 grid with 9 positions\n\nIf position is row 2 column 3, Enter 6 as input\n\nEnter your position for 'o': ");
    }
    scanf("%d",&TEMP2);
  }
  if(1==turn)
  {
    switch (TEMP2)
    {
      case 1:GRID[1][1]='x';
      break;
      case 2:GRID[1][3]='x';
      break;
      case 3:GRID[1][5]='x';
      break;
      case 4:GRID[3][1]='x';
      break;
      case 5:GRID[3][3]='x';
      break;
      case 6:GRID[3][5]='x';
      break;
      case 7:GRID[5][1]='x';
      break;
      case 8:GRID[5][3]='x';
      break;
      case 9:GRID[5][5]='x';
      break;
    }
 }
  else
  {
    switch (TEMP2)
    {
      case 1:GRID[1][1]='o';
      break;
      case 2:GRID[1][3]='o';
      break;
      case 3:GRID[1][5]='o';
      break;
      case 4:GRID[3][1]='o';
      break;
      case 5:GRID[3][3]='o';
      break;
      case 6:GRID[3][5]='o';
      break;
      case 7:GRID[5][1]='o';
      break;
      case 8:GRID[5][3]='o';
      break;
      case 9:GRID[5][5]='o';
      break;
  }
 }
}
//function to find the number of available available_spots
int available_spots(char (*GRID)[7])
{
  int spots=0;
  for(int i=1;i<6;i=i+2)
  for(int j=1;j<6;j=j+2)
  {
    if (GRID[i][j]==' ')
    spots++;
  }
  return spots;
}
//function to check for win condition after every move from user and computer
int terminal_state(char (*GRID)[7])
{
 if (((GRID[1][1]=='o')&&(GRID[1][3]=='o'))&&((GRID[1][3]=='o')&&(GRID[1][5]=='o')))
 {
   return -10;
 }
 else  if (((GRID[3][1]=='o')&&(GRID[3][3]=='o'))&&((GRID[3][3]=='o')&&(GRID[3][5]=='o')))
 {
   return -10;
 }
 else  if (((GRID[5][1]=='o')&&(GRID[5][3]=='o'))&&((GRID[5][3]=='o')&&(GRID[5][5]=='o')))
 {
   return -10;
 }
 else  if (((GRID[1][1]=='o')&&(GRID[3][1]=='o'))&&((GRID[3][1]=='o')&&(GRID[5][1]=='o')))
 {
   return -10;
 }
 else  if (((GRID[1][3]=='o')&&(GRID[3][3]=='o'))&&((GRID[3][3]=='o')&&(GRID[5][3]=='o')))
 {
   return -10;
 }
 else  if (((GRID[1][5]=='o')&&(GRID[3][5]=='o'))&&((GRID[3][5]=='o')&&(GRID[5][5]=='o')))
 {
   return -10;
 }
 else  if (((GRID[1][1]=='o')&&(GRID[3][3]=='o'))&&((GRID[3][3]=='o')&&(GRID[5][5]=='o')))
 {
   return -10;
 }
 else  if (((GRID[1][5]=='o')&&(GRID[3][3]=='o'))&&((GRID[3][3]=='o')&&(GRID[5][1]=='o')))
 {
   return -10;
 }
 if (((GRID[1][1]=='x')&&(GRID[1][3]=='x'))&&((GRID[1][3]=='x')&&(GRID[1][5]=='x')))
 {
   return 10;
 }
 else  if (((GRID[3][1]=='x')&&(GRID[3][3]=='x'))&&((GRID[3][3]=='x')&&(GRID[3][5]=='x')))
 {
   return 10;
 }
 else  if (((GRID[5][1]=='x')&&(GRID[5][3]=='x'))&&((GRID[5][3]=='x')&&(GRID[5][5]=='x')))
 {
   return 10;
 }
 else  if (((GRID[1][1]=='x')&&(GRID[3][1]=='x'))&&((GRID[3][1]=='x')&&(GRID[5][1]=='x')))
 {
   return 10;
 }
 else  if (((GRID[1][3]=='x')&&(GRID[3][3]=='x'))&&((GRID[3][3]=='x')&&(GRID[5][3]=='x')))
 {
   return 10;
 }
 else  if (((GRID[1][5]=='x')&&(GRID[3][5]=='x'))&&((GRID[3][5]=='x')&&(GRID[5][5]=='x')))
 {
   return 10;
 }
 else  if (((GRID[1][1]=='x')&&(GRID[3][3]=='x'))&&((GRID[3][3]=='x')&&(GRID[5][5]=='x')))
 {
   return 10;
 }
 else  if (((GRID[1][5]=='x')&&(GRID[3][3]=='x'))&&((GRID[3][3]=='x')&&(GRID[5][1]=='x')))
 {
   return 10;
 }
 else if(available_spots(GRID)==0)
 return 0;
}
/*function of minimax algorithm that is recursive*/
int minimax(char (*GRID)[7],int turn)
{
  //
  int best;
  if(terminal_state(GRID)==10)
  return 10;
  if(terminal_state(GRID)==-10)
  return -10;
  if(terminal_state(GRID)==0)
  return 0;
  //
  if(0==turn)
  {
    int i,j;
    int temp;
    best=1000;
    for(int i=1;i<6;i=i+2)
    {
      for(int j=1;j<6;j=j+2)
      {
        if(GRID[i][j]==' ')
        {
          GRID[i][j]='o';
          temp=minimax(GRID,1);
          best=MIN(temp,best);
          GRID[i][j]=' ';
        }
      }
    }
    return best;
   }
  else if(1==turn)
  {
    int i,j;
    int temp;
    best=-1000;
      for(i=1;i<6;i=i+2)
      {
        for(j=1;j<6;j=j+2)
        {
          if(GRID[i][j]==' ')
          {
            GRID[i][j]='x';
            temp=minimax(GRID,0);
            best=MAX(temp,best);
            GRID[i][j]=' ';
          }
        }
      }
  return best;
  }
}
//the following function returns the best move
void computer_position(char (*GRID)[7],int turn)
{
  if(0==turn)
  {
    int sample=1000;
    int computer_move[3];
    computer_move[0]=sample;
    for(int i=1;i<6;i=i+2)
    {
      for(int j=1;j<6;j=j+2)
      {
        if(GRID[i][j]==' ')
        {
          GRID[i][j]='o';
          sample=minimax(GRID,!turn);
          if(sample<computer_move[0])
          {
            computer_move[0]=sample;
            computer_move[1]=i;
            computer_move[2]=j;
          }
          GRID[i][j]=' ';
        }
      }
    }
    GRID[computer_move[1]][computer_move[2]]='o';
  }
  else
  {
    int sample=-1000;
    int computer_move[3];
    computer_move[0]=sample;
    for(int i=1;i<6;i=i+2)
    {
      for(int j=1;j<6;j=j+2)
      {
        if(GRID[i][j]==' ')
        {
          GRID[i][j]='x';
          sample=minimax(GRID,!turn);
          if(sample>computer_move[0])
          {
            computer_move[0]=sample;
            computer_move[1]=i;
            computer_move[2]=j;
          }
          GRID[i][j]=' ';
        }
      }
    }
    GRID[computer_move[1]][computer_move[2]]='x';
  }
}
/*main function*/
int main()
{
 //int USR_INPUT,RESULT;
 char GRID[7][7]={
      {'-','-','-','-','-','-','-'},
      {'|',' ','|',' ','|',' ','|'},
      {'-','-','-','-','-','-','-'},
      {'|',' ','|',' ','|',' ','|'},
      {'-','-','-','-','-','-','-'},
      {'|',' ','|',' ','|',' ','|'},
      {'-','-','-','-','-','-','-'}
 };
 int turn;
 //get the turn from the user
 printf("\nDo you want to go first as 'x', or second as 'o'?\n");
 printf("\nEnter 1 for 'x' or 0 for 'o':");
 scanf("%d",&turn);
 while((turn!=0)&&(turn!=1))
 {
   printf("\nInvalid value for turn!\n");
   printf("\nDo you want to go first as 'x', or second as 'o'?\n");
   printf("\nEnter 1 for 'x' or 0 for 'o':");
   scanf("%d",&turn);
 }
 //when no spots are available or a win condition is obtained
 if(1==turn)
 {
   while(available_spots(GRID)!=0)
   {
   print(GRID);
   get_input(GRID,turn);
   if(terminal_state(GRID)==10)
   {
     print(GRID);
     printf("\nYou have won\n\n");
     return 0;
   }else if (terminal_state(GRID)==-10) {
     print(GRID);
     printf("\nComputer has won\n\n");
     return 0;
   }else if (terminal_state(GRID)==0) {
     print(GRID);
     printf("\nThe game is a draw\n\n");
     return 0;
   }
   computer_position(GRID,!turn);
   }
 }
 else
 {
   while(available_spots(GRID)!=0)
   {
     computer_position(GRID,!turn);
     if(terminal_state(GRID)==-10)
     {
       print(GRID);
       printf("\nYou have won\n\n");
       return 0;
     }else if (terminal_state(GRID)==10) {
       print(GRID);
       printf("\nComputer has won\n\n");
       return 0;
     }else if (terminal_state(GRID)==0) {
       print(GRID);
       printf("\nThe game is a draw\n\n");
       return 0;
     }
     print(GRID);
     get_input(GRID,turn);
   }
 }
}
\$\endgroup\$
6
\$\begingroup\$

Please add some space to your code - it can be hard to read when it's all crammed together. For example, compare these two versions of the same function; first, as written:

//function to find the number of available available_spots
int available_spots(char (*GRID)[7])
{
  int spots=0;
  for(int i=1;i<6;i=i+2)
  for(int j=1;j<6;j=j+2)
  {
    if (GRID[i][j]==' ')
    spots++;
  }
  return spots;
}

Secondly, after reindenting and adding some space for the operators to breathe:

// Count the number of empty positions
int available_spots(char grid[][7])
{
    int spots = 0;

    for (int i = 1;  i < 6;  i += 2) {
        for (int j = 1;  j < 6;  j += 2) {
            if (grid[i][j] == ' ') {
                ++spots;
            }
        }
    }

    return spots;
}

Use ALL_CAPS names for preprocessor macros, but not for ordinary variables or functions. It's important to be able to tell them apart (because macros don't obey the rules of scope, for instance).


The game grid is 7✕7 = 49 elements, but only 3✕3 = 9 of those elements are actually storing game state - that's only ⅕. The rest are filled with presentation constants that never need to be changed, and can be produced when printing (they don't need to be stored).


Look at this repetition:

    case 1:
          if (GRID[1][1]=='x'||GRID[1][1]=='o')
          {
            printf("\nPosition already occupied!!!\n");
            return 0;
          }
          else
            return 1;
    break;
    case 2:
          if (GRID[1][3]=='x'||GRID[1][3]=='o')
          {
            printf("\nPosition already occupied!!!\n");
            return 0;
          }
          else
            return 1;
    break;

Here, we're doing two things: mapping the input number to a position, and checking whether it's occupied. If we separate those computations, we'll find we don't need to repeat the tests.

Something like this:

if (input < 1 || input > 9) {
    /* out of range */
    return 0;
}

--input;   /* now 0-8 */
int x = input % 3;
int y = input / 3;

char occupier = grid[2*y+1][2*x+1];

if (occupier != ' ') {
    printf("\nPosition already occupied!!!\n");
    return 0;
} else {
    return 1;
}

Similar techniques can reduce duplication elsewhere in the program.


Enable more warning messages in your compiler. It will help you spot errors such as these:

gcc -std=c17 -fPIC -g -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds      -Wconversion    214336.c    -o 214336
214336.c: In function ‘minimax’:
214336.c:287:11: warning: unused variable ‘j’ [-Wunused-variable]
     int i,j;
           ^
214336.c:287:9: warning: unused variable ‘i’ [-Wunused-variable]
     int i,j;
         ^
214336.c: In function ‘valid’:
214336.c:114:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
214336.c: In function ‘terminal_state’:
214336.c:272:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
214336.c: In function ‘minimax’:
214336.c:325:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
\$\endgroup\$
  • \$\begingroup\$ Can I add more space using the comments, such as /******************/, or just by entering space? \$\endgroup\$ – Udhay Sankar Feb 27 at 4:57
  • \$\begingroup\$ @UdhaySankar use empty new lines to logically separate content. \$\endgroup\$ – bhathiya-perera Feb 27 at 9:04
  • \$\begingroup\$ @Udhay, I've edited with a simple example; does that help? \$\endgroup\$ – Toby Speight Feb 27 at 9:07

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.