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;
}

Link to complete code:
https://pastebin.com/BFULPtJB

I'd like sugestions on how to improve it.

\$\endgroup\$
2
  • 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\$ – Neil Feb 2 at 23:36
  • \$\begingroup\$ Please be aware that we can review only the code in the question. If you want a review of the whole program, it needs to be shown directly in the question, not hidden behind a link. \$\endgroup\$ – Toby Speight Feb 3 at 8:47
1
\$\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\$

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.