A beginner's implementation of tic-tac-toe

Question

I recently tried to make a tic-tac-toe game with my knowledge. I haven't implemented any fancy algorithms like minimax. I made it entirely with my might. As a result, I haven't been able to beat the computer even once. I was wondering what experts would think of the following implementation of the game. I wish you all a fabulous day! :)

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

char block[3][3];
const char PLAYER = 'X';
const char COMPUTER = 'O';

// functions
void newBlock();
void updateBlock();
void userInput();
void randomInput();
void fstCompInput();
void printWinner(char winnerName);
void computerInput();
void repeatInputs();
int freeSpaces();
int check(char checkType, int pos, char playerType);
char winner();
bool validMove(int row, int col);
bool canWin(char winnerType);

/* algorithm of this game
1. make GUI of block
2. update block with spaces
3. keep doing these until game finishes ;
        take user input
            take rows and cols
            insert 'x' at that place only if empty
        take computer's input
            find the most optimal move 
                if computer can win, make the winning move
                else block the user from winning
            else just move randomly
        update block
        if somebody wins or if game has tied ( end game )
        else repeat inputs (goto step 3)
end */


// MAIN
int main(void)
{
    // seeding time
    srand(time(0));
    
    newBlock();
    updateBlock();

    // first round
    userInput();
    fstCompInput();
    updateBlock();

    do
    {
        repeatInputs();
    }
    while(winner() == ' ');         // nobody has won, so play another round

    // somebody other than ' ' has won
    printWinner(winner());
}

void userInput()
{
    // prompt user for his move
    int row, col;
    printf("Where do you want X ? (#row-#col) ");
    scanf("%d-%d", &row,&col);
    row--; col--;

    if(validMove(row, col))
        block[row][col] =  PLAYER;
    else
    {
        printf("\nTHAT'S INVALID!!!\n");
        userInput();
    }
}

void fstCompInput()
{
    // if the player didn't take the middle, well, we'll take it.
    // if the player took the middle, a move at corner sounds good.
    if(block[1][1] == ' ') {
        block[1][1] = COMPUTER;
        return;
    }
    int cornerIndices[4][2] = {{0,0},{0,2},{2,0},{2,2}};
    int randCorner = rand() % 4;
    int idx1 = cornerIndices[randCorner][0];
    int idx2 = cornerIndices[randCorner][1];
    block[idx1][idx2] == ' ' ? block[idx1][idx2] = COMPUTER: fstCompInput();
    
}

void randomInput()
{
    int randRow = rand() % 3;
    int randCol = rand() % 3;

    if(block[randRow][randCol] == ' ')
        block[randRow][randCol] = COMPUTER;
    else if(freeSpaces() == 0)
        return;
    else{
        randomInput();
    }
}

void computerInput()
{
    if(canWin(COMPUTER)) return;
    if(canWin(PLAYER)) return;
    // both can't win then,
    randomInput();
}

int check(char checkType, int pos, char playerType)
{
    char arr[3];
    // check for rows
    if(checkType == 'r')
        for(int i = 0; i < 3; i++)
            arr[i] = block[pos][i];    

    // check for cols
    if(checkType == 'c')
        for(int i = 0; i < 3; i++)
            arr[i] = block[i][pos];

    // check for diags
    if(checkType == 'm')                      // main diagonal(top left to bottom right)
        for(int i = 0; i < 3; i++)
            arr[i] = block[i][i];
    else if(checkType == 'o')                   // other diagonal
        for(int i = 0; i < 3; i++)
            arr[i] = block[i][2-i];
   
    bool zero = (arr[1] == playerType && arr[2] == playerType && arr[0] == ' ');
    bool one = (arr[0] == playerType && arr[2] == playerType && arr[1] == ' ');
    bool two = (arr[0] == playerType && arr[1] == playerType && arr[2] == ' ');

    if(zero) return 0;
    if(one) return 1;
    if(two) return 2;
    return -1;          // no right place found  
}

char winner()
{
    // matching rows
    for(int row = 0; row < 3; row++)
        if(block[row][0] == block[row][1] && block[row][0] == block[row][2])
            return block[row][0];

    // matching columns
    for(int col = 0; col < 3; col++)
        if(block[0][col] == block[1][col] && block[0][col] == block[2][col])
            return block[0][col];

    // matching diagonals
    if ( (block[0][0] == block[1][1] && block[0][0] == block[2][2]) || (block[0][2] == block[1][1] && block[0][2] == block[2][0]) )
        return block[1][1];

    // no winner until the end of all freespaces
    else if(freeSpaces() == 1)
        return 'T';             // game has tied
    else
        return ' ';            //  nobody wins
}

void repeatInputs()
{
    userInput();
    computerInput();
    updateBlock();
}

bool canWin(char playerType)
{
    int goodIndex;

    // go through every row
    for(int thisRow = 0; thisRow < 3; thisRow++)
    {
        // check if this row can make user win
        goodIndex = check('r', thisRow, playerType);
        if(goodIndex != -1) {
            block[thisRow][goodIndex] = COMPUTER;
            return true;
        }
    }
    
    // go through every col
    for(int thisCol = 0; thisCol < 3; thisCol++)
    {
        // check if this col can make user win
        goodIndex = check('c', thisCol, playerType);
        if(goodIndex != -1) {
            block[goodIndex][thisCol] = COMPUTER;
            return true;
        } 
    }
    // check diagonals
    goodIndex = check('m', 0, playerType);           // checking for main diagonal
    if(goodIndex != -1){
        block[goodIndex][goodIndex] = COMPUTER;
        return true;
    }
    goodIndex = check('o', 0, playerType);           // checking another diagonal
    if(goodIndex != -1){
        block[goodIndex][2-goodIndex] = COMPUTER;
        return true;
    }

    // if nothing matching
    return false;
}

void newBlock()
{
    for(int i = 0; i < 3; i++)
        for(int j = 0; j < 3; j++)
            block[i][j] = ' ';
}


void updateBlock()
{
    printf("\n------------------------------------------------------------------------\n\n");
    printf("\t\t   ____________________\n");
    printf("\t\t  |  ________________  |\n");
    printf("\t\t  | |                | |\n");
    printf("\t\t  | |  #  1  2  3    | |\n");
    printf("\t\t  | |  1 %c | %c | %c   | |\n", block[0][0], block[0][1], block[0][2]);
    printf("\t\t  | |   ---|---|---  | |\n");
    printf("\t\t  | |  2 %c | %c | %c   | |\n", block[1][0], block[1][1], block[1][2]);
    printf("\t\t  | |   ---|---|---  | |\n");
    printf("\t\t  | |  3 %c | %c | %c   | |\n", block[2][0], block[2][1], block[2][2]);
    printf("\t\t  | |________________| |\n");
    printf("\t\t  |____________________|\n");
    printf("\n\n------------------------------------------------------------------------\n\n");
}

int freeSpaces()
{
    int spaces = 9;
    for(int i = 0; i < 3; i++)
    {
        for(int j = 0; j < 3; j++)
        {
            if(block[i][j] != ' ')
                spaces--;
        }
    }
    return spaces;
}

bool validMove(int row, int col)
{
    return (block[row][col] == ' ' && row < 3 && row >= 0 &&  col < 3 && col >= 0);
}

void printWinner(char winnerName)
{
    if (winnerName == 'O')
        printf("COMPUTER WINS!!!");
    else if (winnerName == 'X')
        printf("YOU WIN!!!");
    else
        printf("TIE!");
    
    // ask if he wants to play again
    printf("\n\nPlay Again (y/n) ? "); char c = getche();
    if(c == 'y')
    {
        system("cls");
        main();
    }
    else
    {
        printf("\n\nThanks for playing! \n\n");
        return;
    }
    
}

Answer 1

• #include <conio.h> is an obsolete, non-standard lib which was mostly used for MS DOS programming. If you have need for it, you should probably be using either ncurses (*nix) or the Windows colsole API instead.
• char block[3][3]; don't use global variables. This should probably have been wrapped up in a struct and passed as parameters to all functions.
• void newBlock(); etc. Declaring a function with empty parenthesis is obsolete style. You should use void newBlock (void).
• You should check the result from scanf to see that it was successful. And you should check that the user input was even in range of the board, or you'll access the array out of bounds.
• Don't use assignment inside ?:, it makes the code hard to read and could lead to precedence mistakes. Idiomatic use of ?: should rather look along the lines of a = (cond) ? b : c; with the assignment to the far left of the expression.
• block[idx1][idx2] == ' ' ? block[idx1][idx2] = COMPUTER: fstCompInput(); is not even valid C and will not compile. The 2nd and 3rd expression of ?: need to be of the same type. There's absolutely no reason to use recursion here either, all it does is to slow down the code, make it less readable and creates needless stack use peaks. Just use a loop instead.
• Overall, there doesn't exist a single, valid use-case where a beginner should ever use recursion. It's a pox encountered in way too many beginner classes. You need to remove all recursion from this program, it's just dangerous bloat that needlessly slows the program down for no benefits what-so-ever.
• Similarly, calling main() recursively is horrible practice for all these reasons too. Never do that, use loops.

Answer 2

Firstly let me say this is not bad code. I spend my day on stack overflow answering c questions and this is way way better than most i see.

But posting to CodeReview site takes strong nerves. People are trying to help you improve , but any help they provide will mainly be in pointing out errors

Good points

• good strong naming
• variables declared when first needed not at the top of the function
• variables (mainly) given values when declared if needed
• well laid out, all functions declared together

In no particular order

There is no need for special case in the main loop

// first round
// userInput();
 // fstCompInput();
 // updateBlock();

do
{
    repeatInputs();
}
while(winner() == ' ');         

it works fine like that

Do not use recursion like this

void userInput()
{
    // prompt user for his move
    int row, col;
    printf("Where do you want X ? (#row-#col) ");
    scanf("%d-%d", &row, &col);
    row--; col--;

    if (validMove(row, col))
        block[row][col] = PLAYER;
    else
    {
        printf("\nTHAT'S INVALID!!!\n");
        userInput(); <<<<==================
    }
}

You do not check to see if the user entered valid numeric data either

do this

void userInput()
{
    // prompt user for his move
    int row = 0;
    int col = 0;
    printf("Where do you want X ? (#row-#col) ");
    for(;;){
      scanf("%d-%d", &row, &col);
      row--; col--;

      if (validMove(row, col)){
        block[row][col] = PLAYER;
        return;
      }
      printf("\nTHAT'S INVALID!!!\n");
      // clear the garbage out
      int c;
      while ((c = getchar()) != '\n' && c != EOF);
    }
}

ie loop till you have good input. I would have called this function makeUserMove since it actually updates the board, it doesnt just read the users input.

Note that you do not check if the user entered a number between 1 and 3 always check the return value from system calls like scanf

the function fstCompINput is never used - take it out

More bad recursion

void randomInput()
{
   
    int randRow = rand() % 3;
    int randCol = rand() % 3;

    if (block[randRow][randCol] == ' ')
        block[randRow][randCol] = COMPUTER;
    else if (freeSpaces() == 0)
        return;
    else {
        randomInput();
    }
}

should be

void randomInput()
{
    for(;;){
      int randRow = rand() % 3;
      int randCol = rand() % 3;

      if (block[randRow][randCol] == ' ')
          block[randRow][randCol] = COMPUTER;
      else if (freeSpaces() == 0)
        return;
    }
}

I would do the freespaces check at the start of this function. I would call it 'randomComputerMove' , theres no input involved

in check the if == r, if = c etc is crying out for a switch statement

switch(checktype){ 
    case 'r':
 ....
    case 'c':

the end of check is a bit messy. I would do

 if  (arr[1] == playerType && arr[2] == playerType && arr[0] == ' ')
   return 0;

etc.

And certainly dont do that recursive main call, thats terrible. Simply have a loop in main

Good Luck!