6
\$\begingroup\$

I recently started learning C, though I have some experience in other languages. I recently wrote a TicTacToe AI using the Minimax Algorithm in C. I would like to know how I could write better C.

#include <stdio.h>
#define X -1
#define O -2
#define MAX_SIZE 50 //This is the Max size of the Input Buffer
int turn = O; 
int board[] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
int variations = 0; //To keep track of no. of variations the ai has seen
void copyBoard(int from[], int to[]){
 for(int i = 0; i < 9; i++){
 to[i] = from[i];
 }
}
//gets line WITHOUT \n 
int getl(char s[], int lim){
 int c, i;
 for(i = 0; i < lim-1 && (c = getchar()) != EOF && c != '\n'; i++)
 s[i] = c;
 
 s[i] = '0円';
 return i;
}
//Converts char[] to int
int bufferToNum(char buffer[]){
 int n = 0;
 for(int i = 0; buffer[i] != '0円'; i++){
 n = 10 * n + buffer[i] - '0';
 }
 return n;
}
//converts the board numbers to char to display
char boardToChar(int i){
 int a = board[i];
 if (a == X){ 
 return 'X';
 } else if (a == O){
 return 'O';
 } else { 
 return a + '0';
 } 
}
//prints board
void printBoard(){
 printf("=============\n| %c | %c | %c |\n-------------\n| %c | %c | %c |\n-------------\n| %c | %c | %c |\n=============\n", boardToChar(0), boardToChar(1), boardToChar(2), boardToChar(3), boardToChar(4), boardToChar(5), boardToChar(6), boardToChar(7), boardToChar(8)); 
}
//alternates turn
void alternateTurn(){
 if (turn == O){
 turn = X; 
 } else if (turn == X){
 turn = O;
 }
}
//returns 1 if draw, return 0 if not a draw
int drawCheck(int l_board[]){
 for(int i = 0; i < 9; i++){
 if (l_board[i] == i+1){
 return 0;
 }
 }
 return 1;
}
//returns X if X won and O if O one and 0 if nobody winning
int winCheck(int l_board[]){
 //Rows
 for (int i = 0; i < 3; i++){
 if (l_board[3*i] == l_board[3*i + 1] && l_board[3*i + 1] == l_board[3*i + 2]){
 return l_board[3*i];
 }
 }
 //Columns
 for (int j = 0; j < 3; j++){
 if (l_board[j] == l_board[3 + j] && l_board[3 + j] == l_board[6 + j]){
 return l_board[j];
 }
 }
 //Diagonal Top Left to Bottom Right
 if (l_board[0] == l_board[4] && l_board[0] == l_board[8]){
 return l_board[0];
 }
 //Diagonal Top Right to bottom Left
 if (l_board[2] == l_board[4] && l_board[2] == l_board[6]){
 return l_board[2];
 }
 return 0;
}
//1 if nothing is ther and 0 if something was already ther
int putInBoard(int l_board[], int pos, int newVal){
 if (l_board[pos] == pos+1){
 l_board[pos] = newVal;
 return 1;
 } else
 {
 return 0;
 } 
}
//X if X win, O if O win, 0 if draw, 1 if nothing
int gameState(int l_board[]){
 int wc = winCheck(l_board);
 if (wc == X){
 return X;
 } else if(wc == O){
 return O;
 } else {
 if (drawCheck(l_board)){
 return 0;
 }
 }
 return 1;
}
void legalMoves(int l_board[], int output[]){ 
 for(int i = 0; i < 9; i++){
 if (l_board[i] == i+1){
 output[i] = 1;
 } else {
 output[i] = 0;
 }
 }
}
int max(int a, int b){
 return a>b ? a : b;
}
int min(int a, int b){
 return a<b ? a : b;
}
//X is ai
int minimax(int l_board[], int depth, int maximising){
 int gs = gameState(l_board);
 variations++;
 if (gs == X){
 return 10;
 } else if (gs == O){
 return -10;
 } else if (gs == 0){
 return 0;
 }
 if (depth == 0){
 return 0;
 }
 if (maximising){
 //Its AI's Turn so it has to maximise
 int val = -100;
 int legalMovesArr[9];
 legalMoves(l_board, legalMovesArr);
 for (int i = 0; i < 9; i++){
 if (legalMovesArr[i]){
 int tempBoard[9];
 copyBoard(l_board, tempBoard);
 putInBoard(tempBoard, i, X);
 val = max(minimax(tempBoard, depth-1, 0), val);
 }
 }
 return val;
 } else { 
 int val = 100;
 int legalMovesArr[9];
 legalMoves(l_board, legalMovesArr);
 for (int i = 0; i < 9; i++){
 if (legalMovesArr[i]){
 int tempBoard[9];
 copyBoard(l_board, tempBoard);
 putInBoard(tempBoard, i, O);
 val = min(minimax(tempBoard, depth-1, 1), val);
 }
 }
 return val;
 }
}
int ai(int l_board[], int depth){
 int legalMovesArr[9];
 legalMoves(board, legalMovesArr);
 int val = -100; 
 int best_move = 0;
 for (int i = 0; i < 9; i++){
 if (legalMovesArr[i]){
 int tempBoard[9];
 copyBoard(l_board, tempBoard);
 putInBoard(tempBoard, i, X);
 int temp = minimax(tempBoard, depth-1, 0);
 if (val <= temp){
 val = temp;
 best_move = i;
 } 
 }
 }
 return best_move;
}
int main(){
 printBoard();
 int gameOn = 0;
 char buffer[MAX_SIZE];
 while(!gameOn){
 if (turn == O){
 printf("%c's turn: ", turn == X ? 'X' : 'O');
 getl(buffer, MAX_SIZE);
 int num = bufferToNum(buffer); 
 while (num <= 0 || num > 9){
 printf("Please enter an integer between 1 and 9: ");
 getl(buffer, MAX_SIZE);
 num = bufferToNum(buffer); 
 }
 if (putInBoard(board, num-1, turn)){
 ;
 } else { 
 while(!putInBoard(board, num-1, turn)){
 printf("Something already exists, Please enter a new number: ");
 getl(buffer, MAX_SIZE);
 num = bufferToNum(buffer);
 }
 }
 } else {
 putInBoard(board, ai(board, 8), X);
 printf("Calculated %d variations\n", variations);
 variations = 0;
 }
 
 printBoard(); 
 alternateTurn();
 int gs = gameState(board);
 if (gs == X){
 printf("X won!");
 return 0;
 } else if (gs == O){
 printf("O won!");
 return 0;
 } else if (gs == 0){
 printf("Draw!");
 return 0;
 } 
 }
 
 return 0;
}
asked Aug 18, 2020 at 15:44
\$\endgroup\$

2 Answers 2

9
\$\begingroup\$

General Observations

I see a lot of good programming practices followed here already, keep up the good work.

Most of the functions are small and follow the Single Responsibility Principle.

Use Library Functions When You Can

The function getl() can be implemented using 2 standard C library functions, fgets and strrchr.

The fgets() function inputs an entire line of characters at a time, rather than using character input, and the strrchr() will allow you to find the \n character and replace it.

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code multiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

The repetition occurs in the function int minimax(int l_board[], int depth, int maximising) :

 if (maximising) {
 //Its AI's Turn so it has to maximise
 int val = -100;
 int legalMovesArr[9];
 legalMoves(l_board, legalMovesArr);
 for (int i = 0; i < 9; i++) {
 if (legalMovesArr[i]) {
 int tempBoard[9];
 copyBoard(l_board, tempBoard);
 putInBoard(tempBoard, i, X);
 val = max(minimax(tempBoard, depth - 1, 0), val);
 }
 }
 return val;
 }
 else {
 int val = 100;
 int legalMovesArr[9];
 legalMoves(l_board, legalMovesArr);
 for (int i = 0; i < 9; i++) {
 if (legalMovesArr[i]) {
 int tempBoard[9];
 copyBoard(l_board, tempBoard);
 putInBoard(tempBoard, i, O);
 val = min(minimax(tempBoard, depth - 1, 1), val);
 }
 }

This repeating code could be it's own function.

The repeating code also makes the function too complex.

Complexity

The complexity of the function minimax() was mentioned above, the function main() is also too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

The entire while (!gameOn) loop should be within its own function.

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.

Update Answer to comment

void game_loops()
{
 if (turn == O) {
 printf("%c's turn: ", turn == X ? 'X' : 'O');
 char buffer[MAX_SIZE];
 getl(buffer, MAX_SIZE);
 int num = bufferToNum(buffer);
 while (num <= 0 || num > 9) {
 printf("Please enter an integer between 1 and 9: ");
 getl(buffer, MAX_SIZE);
 num = bufferToNum(buffer);
 }
 if (putInBoard(board, num - 1, turn)) {
 ;
 }
 else {
 while (!putInBoard(board, num - 1, turn)) {
 printf("Something already exists, Please enter a new number: ");
 getl(buffer, MAX_SIZE);
 num = bufferToNum(buffer);
 }
 }
 }
 else {
 putInBoard(board, ai(board, 8), X);
 printf("Calculated %d variations\n", variations);
 variations = 0;
 }
 printBoard();
 alternateTurn();
}
int main() {
 printBoard();
 int gs = 1;
 while (gs == 1) {
 game_loops();
 gs = gameState(board);
 }
 switch (gs)
 {
 case X:
 printf("X won!");
 break;
 case O:
 printf("O won!");
 break;
 default:
 printf("Draw!");
 break;
 }
 return 0;
}
answered Aug 18, 2020 at 17:27
\$\endgroup\$
3
  • \$\begingroup\$ First of all, thanks for your answer. I have few comments and questions though. 1)I understand your point of getl and I would use the standard library functions in actual code, but I wanted to do low-level stuff and implement my own functions for these tasks. 2) How would copying the whole main() code to a function help in readability? 3)I think I had minimal global variables, what variables should not have been in the global scope? Thank you for your time. \$\endgroup\$ Commented Aug 18, 2020 at 17:38
  • 4
    \$\begingroup\$ The minimal and desired number of global variables is zero, nothing should be done by side affects. This program is very easy to write without global variables. I've updated the answer to show how it readable the loop would be, but every removal of indentation helps readability. You don't want to use character by character input when you can use buffered input instead for performance reasons. \$\endgroup\$ Commented Aug 18, 2020 at 18:10
  • 1
    \$\begingroup\$ "getl() can be implemented using 2 standard C library functions" --> OP's getl() has an obscure advantage: it returns the correct number of characters read when one of them is surprisingly a null character. fgets()/strrchr() needs more to support that rare need. Also the fgets() approach needs a +1 size buffer. \$\endgroup\$ Commented Aug 19, 2020 at 5:48
4
\$\begingroup\$

Small review

Break up that long line of code.

//prints board
void printBoard(){
 printf("=============\n| %c | %c | %c |\n-------------\n| %c | %c | %c |\n-------------\n| %c | %c | %c |\n=============\n", boardToChar(0), boardToChar(1), boardToChar(2), boardToChar(3), boardToChar(4), boardToChar(5), boardToChar(6), boardToChar(7), boardToChar(8)); 
}

Perhaps as

//prints board
void printBoard() {
 printf("=============\n"
 "| %c | %c | %c |\n"
 "-------------\n"
 "| %c | %c | %c |\n"
 "-------------\n"
 "| %c | %c | %c |\n"
 "=============\n", //
 boardToChar(0), boardToChar(1), boardToChar(2), //
 boardToChar(3), boardToChar(4), boardToChar(5), // 
 boardToChar(6), boardToChar(7), boardToChar(8)); 
}

For me, the // at the end of 3 lines prevent auto formatting of those lines into 1 or 2 lines.

answered Aug 19, 2020 at 5:57
\$\endgroup\$
3
  • \$\begingroup\$ Yes, that looks much better. I remember from K&R that we can do "hello" " world" but I didn't think of it while coding. Thanks! \$\endgroup\$ Commented Aug 19, 2020 at 7:09
  • \$\begingroup\$ Wouldn't a printf for each line be cleaner and easier to maintain? \$\endgroup\$ Commented Aug 24, 2020 at 10:09
  • \$\begingroup\$ @CacahueteFrito Versu OP's code, "printf for each line" sounds like an interesting alternative to this answer's re-format only approach. Perhaps post as an answer? For me, 6.01 vs. half-a-dozen of the other - a difference, but not much. \$\endgroup\$ Commented Aug 24, 2020 at 14:19

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.