3
\$\begingroup\$

I 've made a tic-tac-toe game in C, I want some tips on how to improve it since the program is a little bit too long.
(500 lines with comments)
How I done the game:
Mapped a 3x3 Matrix like the board bellow: 

/*
-------------------------
|   1   ||  2   ||  3   |
-------------------------
|   4   ||  5   ||  6   |
-------------------------
|   7   ||  8   ||  9   |
-------------------------
*/

With the 3x3 i did a switch with the position, as show in the code snippets bellow I think that this solution is a little bit too long, so as the algorithms to calculate the win on a given column, row or diagonal.

Here I will show some snipets of the code:

/*
  is_pos_free takes as arg1 a square matrix (board), state (from the enum on Main), and a position (from 1 trought 9)
  return:
  return 1 if position is free
  return 0 if position isn't free
  return -1 if position isn't within range of 1 until 9 (position < 1 or position > 9)
*/
int is_pos_free(int board[SIZE][SIZE],int state,int pos){
  int cond;
  switch(pos){
      case 1: if(state == 0  && board[0][0] == 0 ){
                board[0][0] = 1;
                cond = 1;
                break;
              }
              else if(state == 1 && board[0][0] == 0){
                board[0][0] = 2;
                break;
              }
              else{
                cond = 0;
                break;
              }

      case 2: if(state == 0  && board[0][1] == 0){
                board[0][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[0][1] == 0){
                board[0][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 3: if(state == 0  && board[0][2] == 0){
                board[0][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[0][2] == 0){
                board[0][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 4: if(state == 0  && board[1][0] == 0){
                board[1][0] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][0] == 0){
                board[1][0] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 5: if(state == 0  && board[1][1] == 0){
                board[1][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][1] == 0){
                board[1][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 6: if(state == 0  && board[1][2] == 0){
                board[1][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][2] == 0){
                board[1][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 7: if(state == 0  && board[2][0] == 0){
                board[2][0] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][0] == 0){
                board[2][0] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 8: if(state == 0  && board[2][1] == 0){
                board[2][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][1] == 0){
                board[2][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 9: if(state == 0  && board[2][2] == 0){
                board[2][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][2] == 0){
                board[2][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }
      default:
              printf("%s",RED);
              printf("Error 2\n");
              printf("%s",YELLOW);
              printf("Position <1 or Position > 9\nType a valid position\n");
              cond = -1;
              break;
  }
  return cond;

}

/*
  calculate_win_line takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a line
  return:
  returns 1 for 'O' won
  returns 2 for 'X' won
  default 0 -> No one won
*/

int calculate_win_line(int board[][SIZE],int size){
  int j;
  int k;
  int win_count_O ;
  int win_count_X ;
  int who_won = 0;
  for(j = 0;j<size;j++){
    win_count_O = 0;
    win_count_X = 0;
    for(k = 0;k<size;k++){

      if(board[j][k] == 1){
        win_count_O++;
      }

      else if(board[j][k] == 2){
        win_count_X++;
      }
      if(win_count_O == 3){
        who_won = 1;
        break;
      }

      else if(win_count_X == 3 ){
        who_won = 2;
        break;
      }

    }
  }
  return who_won;
}

/*
  calculate_win_col takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a column
  return:
  returns 1 for 'O' won
  returns 2 for 'X' won
  default 0 -> No one won
*/

int calculate_win_col(int board[][SIZE],int size){
  int j;
  int k;
  int win_count_O = 0;
  int win_count_X = 0;
  int who_won = 0;


  for(j = 0;j<size;j++){
    win_count_O = 0;
    win_count_X = 0;
    for(k = 0;k<size;k++){

        if(board[k][j] == 1){
          win_count_O++;
        }
        else if(board[k][j] == 2){
          win_count_X++;
        }

        if(win_count_O == 3){
          who_won = 1;
          break;
        }

        if(win_count_X == 3){
          who_won = 2;
          break;
        }

    }
  }
  return who_won;
}

/*
  calculate_win_diagonal takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a main diagonal or on a antidiagonal of the given matrix

  return:
  returns 1 for 'O' won on a main diagonal or antidiagonal
  returns 2 for 'X' won on a main diagonal or antidiagonal
  default 0 -> No one won
*/
int calculate_win_diagonal(int board[][SIZE],int size){

  int j;
  int k;
  int win_count_X = 0;
  int win_count_O = 0;
  int who_won = 0;
  for(j = 0;j<size;j++){
    if(board[j][j] == 1){
      win_count_O++;
    }
    if(board[j][j] == 2){
      win_count_X++;
    }
  }
  if(win_count_O == 3){
    return who_won = 1;
  }
  else if(win_count_X == 3){
    return who_won = 2;
  }
  else{
  win_count_O = 0;
  win_count_X = 0;
  k = size - 1;
  }
  for(j = 0;j<size;j++){
    if(board[j][k] == 1){
      win_count_O++;
    }
    else if(board[j][k] == 2){
      win_count_X++;
    }
    k--;
  }
  if(win_count_O == 3){
    who_won = 1;
  }
  else if(win_count_X == 3){
    who_won = 2;
  }
  return who_won;
}

Full code:

//HUMAN-> O
//MACHINE -> X



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

#define SIZE  3

//Colors
#define RED "\033[1;31m"
#define PURPLE "\033[1;35m"
#define CYAN "\033[1;36m"
#define YELLOW "\033[1;33m"
#define RESET "\033[0m"
#define GREEN "\033[1;32m"
//end Colors definitions

// functions prototypes

void determine_turn(int * turn);
void verify_pos(long int * pos,char * buff,char * end);
void draw_pos(void);
void draw_line(void);
void draw_board(int board[][SIZE],int size);
void restart_board(int board[][SIZE],int size);
int is_pos_free(int board[SIZE][SIZE],int state,int pos);
int calculate_win_line(int board[][SIZE],int size);
int calculate_win_col(int board[][SIZE],int size);
int calculate_win_diagonal(int board[][SIZE],int size);

//end of function prototypes

int main(void){
  srand(time(0));
  enum GameState{O_PLAY,X_PLAY};
  int move = 1;
  char buff [255];
  long int pos;
  char * end;
  int status = 0;
  int state = O_PLAY;
  int board [SIZE][SIZE] = {0};
  int win_line;
  int win_col;
  int win_diagonal;
  int cond = 1;
  while(cond == 1){
      while(move <= 9 ){
            draw_pos();
            switch (state) {
                  case O_PLAY:
                              printf("Turn of %sO%s(YOU)\n",YELLOW,RESET);
                              printf("Type the position you want to play based on the board bellow\n");
                              if(fgets(buff,sizeof buff,stdin) == NULL){
                                  printf("%s",RED);
                                  printf("\nFatal Error\n");
                                  printf("%s",PURPLE);
                                  printf("Error reading position\nExpected a number instead got position = NULL\nTerminating program\n");
                                  exit(1);
                                }
                                buff[strcspn(buff,"\n")] = 0;
                                pos  = strtol(buff,&end,10);
                                break;
                case X_PLAY:
                            printf("Turn of %sX%s\n(MACHINE)",CYAN,RESET);
                            break;
                }//end_switch(state)



            status = is_pos_free(board, state, pos);
            while(status == 0 && state == 0){
                    printf("%s",RED);
                    printf("Position %ld already used\n",pos);
                    printf("%s",RESET);
                    printf("Type a new position\n");
                    if(fgets(buff,sizeof buff,stdin) == NULL){
                        printf("%s",RED);
                        printf("\nFatal Error\n");
                        printf("%s",PURPLE);
                        printf("Error reading position\nExpected a number instead got position = NULL\nTerminating program\n");
                        exit(1);
                      }

                      buff[strcspn(buff,"\n")] = 0;
                      pos  = strtol(buff,&end,10);
                      status = is_pos_free(board, state, pos);

            }//end_while(status == 0 && state == 0)

            while(status == 0 && state  == 1){
                  pos = 1 + rand() % 9;
                  status = is_pos_free(board, state, pos);
            }//end_while(status == 0 && state == 1)


            printf("%s",RESET);
            if(state == 0){
                  state = 1;
            }

            else{
                  state = 0;
            }

            draw_board(board,SIZE);
              if(status){
                    move++;
              }
            win_line = calculate_win_line(board, SIZE);
            win_col = calculate_win_col(board, SIZE);
            win_diagonal = calculate_win_diagonal(board, SIZE);
            if(win_line == 1 || win_col == 1 || win_diagonal == 1){
                  printf("%sO%s won\n",YELLOW,RESET);
                  move = 0;
                  break;
            }

            else if (win_line == 2 || win_col == 2 || win_diagonal == 2){
                  printf("%sX%s won\n",CYAN,RESET);
                  move = 0;
                  break;
            }
    }//end while(move<=9)
      if(move == 10){
              printf("Tie\n");

      }
      printf("Wanna play again? (Type 1 to continue playing or type any other key to exit the game)\n");
      if(fgets(buff,sizeof buff,stdin) == NULL){
            printf("%s",RED);
            printf("\nFatal Error\n");
            printf("%s",PURPLE);
            printf("Error reading position\nExpected a number instead got position = NULL\nTerminating program\n");
            exit(1);
      }
      buff[strcspn(buff,"\n")] = 0;
      cond = strtol(buff,&end,10);
      if(cond == 1){
            move = 1;
            win_line = win_col = win_diagonal = 0;
            restart_board(board,SIZE);
      }

      else{
            return 0;
      }
 }//end while(cond == 1)


}//end main()


/*
  draw_line takes as argument void and draws a line on the screen
  return:
  The function returns nothing
*/
void draw_line(void){

  int j;
  printf("\n");
  for(j = 1;j<= (SIZE * 16) + 1;j++){
    printf("-");
  }
  printf("\n");
}

/*
  draw_pos takes as arguments void and draws the positions of the board (from 1 trought 9)
  return:
  The function returns nothing
*/
void draw_pos(void){

  int i;
  int j;
  int val = 1;

  draw_line();
  for(i = 0;i<SIZE;i++){
    for(j = 0;j<SIZE;j++){
      printf("|\t%d\t|",j + val);
    }
    draw_line();
    val+=3;

  }
}

/*
  draw_board takes as arg1 a square matrix (board), and the size of the board, the function draws the board on the screen
  attetion: Do not mistake with draw_pos, since one draws the position mapping and the other draw the board with O and X
  return:
  The function returns nothing
*/
void draw_board(int board[][SIZE],int size){

  int j;
  int k;
  draw_line();
  for( j = 0;j<size;j++){
    for(k = 0;k<size;k++){
      if(board[j][k] == 1){
        printf("|\t%sO%s\t|",YELLOW,RESET);
      }
      else if(board[j][k] == 2){
        printf("|\t%sX%s\t|",CYAN,RESET);
      }
      else{
        printf("|\t \t|");
      }
    }
    draw_line();
  }
}

/*
  is_pos_free takes as arg1 a square matrix (board), state (from the enum on Main), and a position (from 1 trought 9)
  return:
  return 1 if position is free
  return 0 if position isn't free
  return -1 if position isn't within range of 1 until 9 (position < 1 or position > 9)
*/
int is_pos_free(int board[SIZE][SIZE],int state,int pos){
  int cond;
  switch(pos){
      case 1: if(state == 0  && board[0][0] == 0 ){
                board[0][0] = 1;
                cond = 1;
                break;
              }
              else if(state == 1 && board[0][0] == 0){
                board[0][0] = 2;
                break;
              }
              else{
                cond = 0;
                break;
              }

      case 2: if(state == 0  && board[0][1] == 0){
                board[0][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[0][1] == 0){
                board[0][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 3: if(state == 0  && board[0][2] == 0){
                board[0][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[0][2] == 0){
                board[0][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 4: if(state == 0  && board[1][0] == 0){
                board[1][0] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][0] == 0){
                board[1][0] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 5: if(state == 0  && board[1][1] == 0){
                board[1][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][1] == 0){
                board[1][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 6: if(state == 0  && board[1][2] == 0){
                board[1][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[1][2] == 0){
                board[1][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 7: if(state == 0  && board[2][0] == 0){
                board[2][0] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][0] == 0){
                board[2][0] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 8: if(state == 0  && board[2][1] == 0){
                board[2][1] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][1] == 0){
                board[2][1] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }

      case 9: if(state == 0  && board[2][2] == 0){
                board[2][2] = 1;
                cond = 1;
                break;
              }

              else if(state == 1 && board[2][2] == 0){
                board[2][2] = 2;
                break;
              }

              else{
                cond = 0;
                break;
              }
      default:
              printf("%s",RED);
              printf("Error 2\n");
              printf("%s",YELLOW);
              printf("Position <1 or Position > 9\nType a valid position\n");
              cond = -1;
              break;
  }
  return cond;

}

/*
  calculate_win_line takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a line
  return:
  returns 1 for 'O' won
  returns 2 for 'X' won
  default 0 -> No one won
*/

int calculate_win_line(int board[][SIZE],int size){
  int j;
  int k;
  int win_count_O ;
  int win_count_X ;
  int who_won = 0;
  for(j = 0;j<size;j++){
    win_count_O = 0;
    win_count_X = 0;
    for(k = 0;k<size;k++){

      if(board[j][k] == 1){
        win_count_O++;
      }

      else if(board[j][k] == 2){
        win_count_X++;
      }
      if(win_count_O == 3){
        who_won = 1;
        break;
      }

      else if(win_count_X == 3 ){
        who_won = 2;
        break;
      }

    }
  }
  return who_won;
}

/*
  calculate_win_col takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a column
  return:
  returns 1 for 'O' won
  returns 2 for 'X' won
  default 0 -> No one won
*/

int calculate_win_col(int board[][SIZE],int size){
  int j;
  int k;
  int win_count_O = 0;
  int win_count_X = 0;
  int who_won = 0;


  for(j = 0;j<size;j++){
    win_count_O = 0;
    win_count_X = 0;
    for(k = 0;k<size;k++){

        if(board[k][j] == 1){
          win_count_O++;
        }
        else if(board[k][j] == 2){
          win_count_X++;
        }

        if(win_count_O == 3){
          who_won = 1;
          break;
        }

        if(win_count_X == 3){
          who_won = 2;
          break;
        }

    }
  }
  return who_won;
}

/*
  calculate_win_diagonal takes as arg1 a square matrix (board), and the size of the board
  the function calculate if the player or the machine has won on a main diagonal or on a antidiagonal of the given matrix

  return:
  returns 1 for 'O' won on a main diagonal or antidiagonal
  returns 2 for 'X' won on a main diagonal or antidiagonal
  default 0 -> No one won
*/
int calculate_win_diagonal(int board[][SIZE],int size){

  int j;
  int k;
  int win_count_X = 0;
  int win_count_O = 0;
  int who_won = 0;
  for(j = 0;j<size;j++){
    if(board[j][j] == 1){
      win_count_O++;
    }
    if(board[j][j] == 2){
      win_count_X++;
    }
  }
  if(win_count_O == 3){
    return who_won = 1;
  }
  else if(win_count_X == 3){
    return who_won = 2;
  }
  else{
  win_count_O = 0;
  win_count_X = 0;
  k = size - 1;
  }
  for(j = 0;j<size;j++){
    if(board[j][k] == 1){
      win_count_O++;
    }
    else if(board[j][k] == 2){
      win_count_X++;
    }
    k--;
  }
  if(win_count_O == 3){
    who_won = 1;
  }
  else if(win_count_X == 3){
    who_won = 2;
  }
  return who_won;
}
/*
  the function restart_board takes as arg1 a square matrix (board) and the size of the matrix
  the functions restarts the board to 1
  return:
  the function returns nothing
*/
void restart_board(int board[][SIZE],int size){
  int j;
  int k;

  for(j = 0;j<size;j++){
    for(k = 0;k<size;k++){
          board[j][k] = 0;
    }
  }
}

I'd like sugestions on how to improve it.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ The magic numbers are confusing; instead of an int array, a more human-readable enum { FREE, X, O } would be good for readability. You will have to explain what state and pos are. \$\endgroup\$ Commented Feb 2, 2021 at 23:36

3 Answers 3

Reset to default
2
\$\begingroup\$

I actually created a tic-tac-toe program in C several years ago for the fun of it as I was learning the ins and outs of the C language. Since this post I've updated it and added some additional comments for understanding. Take a look yourself here.

I believe it's redundant to have two functions with nearly identical functionality (determine a win on the current state of the board) separated and essentially return identical information. For example, you have calculate_win_diagonal(..), calculate_win_col(..), and calculate_win_line(..). In my code, I write one win condition function that takes the current state of the board and determines if there is a win condition (3-in-a-row) on the board, regardless if its a row, column or diagonal.

Your algorithm in particular for evaluating a current win on the state of the board is suitable. In the instance you wish to increase the size of the board, your algorithm is dynamic and flexible to the dimensions of the board.

Having a position, pos variable is perfectly acceptable. However, problems rise when using a two-dimensional array. Your game-board is two-dimensional, and using a single variable to describe the position in a two-dimensional array can lead to some problems. You could cut down on a lot of the desired space (especially in your is_pos_free(..) function) by instead using a one-dimensional array for the game-board. This way you don't have to do the necessary calculations to get a pair of positions from a single variable. If you intend on keeping the two-dimensional array for your int board[SIZE][SIZE], you could have a function that looks something like this:

void get_position_pair(int pos /*in-parameter*/ , int* row /*in-out*/, int* col /*in-out*/) { .. }

and calling it by

get_position_pair(pos, &row, &col);

If you're not sure how in-out parameters work, I've attached a link here. Welcome to CR.

\$\endgroup\$
2
\$\begingroup\$
  • Avoid using magic numbers such as comparing move to 9, having status be 0. Use enums instead.
  • is_pos_free uses a lot of hard-coded logic which is repetitive. Use programmatic logic to compute calculate_win_col, calculate_win_line, calculate_win_diagonal in one function. It can probably be done in 30 lines or fewer.
  • I believe scanf is the way to read an integer instead of fgets and strtol.
\$\endgroup\$
1
\$\begingroup\$

I dislike else if where the preceding controlled statement diverted program flow unconditionally (continue/break/return). With a statement like

if (c1)
    modify O  // not touching X
else if (c2)
    modify X  // not touching O

, don't continue

if (c3(O))
    sO
if (c4(X))
    sX

(calculate_win_col/line()):

    switch (board[j][k]) {
    case O_PLAY:
        if INaROW <= ++win_count_O)
            return O_PLAY;
        break;
    case X_PLAY:
        if INaROW <= ++win_count_X)
            return X_PLAY;
        break;
    }
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.