2
\$\begingroup\$

I finally made tic tac toe as a beginner(took a month and a bit) and I want to know if I made things more complicated than they should. It seems like a lot of code for a simple game but I'm not sure. Still learning and I would like some advice if there's something really bad.

PS:I dont know a clear command other than system("cls") so even though I saw people online saying no to it I dont know an alternative.

Code:

#include <iostream>
#include <fstream>
#include <stdio.h>
#include <conio.h>
#include <time.h>
using namespace std;

int aivall, aivaln;
char p1 = ' ', p2 = ' ', p3 = ' ', p4 = ' ', p5 = ' ', p6 = ' ', p7 = ' ', p8 =' ', p9 = ' ', l;
int n, endg = 0, again;

int ai(int a, int b) {
    if (a == 1) {
        switch (b) {
        case 1: {if (p1 == 88 || p1 == 79) {again = 1;} else {p1 = 79;}} break;
        case 2: {if (p2 == 88 || p2 == 79) {again = 1;} else {p2 = 79;}} break;
        case 3: {if (p3 == 88 || p3 == 79) {again = 1;} else {p3 = 79;}} break;
    }}
    if (a == 2) {
        switch (b) {
        case 1: {if (p4 == 88 || p4 == 79) {again = 1;} else {p4 = 79;}} break;
        case 2: {if (p5 == 88 || p5 == 79) {again = 1;} else {p5 = 79;}} break;
        case 3: {if (p6 == 88 || p6 == 79) {again = 1;} else {p6 = 79;}} break;
    }}
    if (a == 3) {
        switch (b) {
        case 1: {if (p7 == 88 || p7 == 79) {again = 1;} else {p7 = 79;}} break;
        case 2: {if (p8 == 88 || p8 == 79) {again = 1;} else {p8 = 79;}} break;
        case 3: {if (p9 == 88 || p9 == 79) {again = 1;} else {p9 = 79;}} break;
    }}

}

char xno(char p1, char p2, char p3, char p4, char p5, char p6, char p7, char p8, char p9) {
    cout << "    1  2  3 " << endl;
    cout << endl;
    cout << " A. " << p1 << "  " << p2 << "  " << p3 << endl;
    cout << endl;
    cout << " B. " << p4 << "  " << p5 << "  " << p6 << endl;
    cout << endl;
    cout << " C. " << p7 << "  " << p8 << "  " << p9 << endl;
    cout << endl;}

int triplecheckx() {
    //vertical
    if (p1 == 88 && p4 == 88 && p7 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
    if (p2 == 88 && p5 == 88 && p8 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
    if (p3 == 88 && p6 == 88 && p9 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }

    //Horizontal
    if (p1 == 88 && p2 == 88 && p3 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
    if (p4 == 88 && p5 == 88 && p6 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
    if (p7 == 88 && p8 == 88 && p9 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }

    //Cross
    if (p1 == 88 && p5 == 88 && p9 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
    if (p3 == 88 && p5 == 88 && p7 == 88) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Player Won!!!";
    }
}

int triplechecko() {
    //vertical
    if (p1 == 79 && p4 == 79 && p7 == 79) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
    if (p2 == 79 && p5 == 79 && p8 == 79) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
    if (p3 == 79 && p6 == 79 && p9 == 79) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }

    //Horizontal
    if (p1 == 79 && p2 == 79 && p3 == 79) {
        endg = 1;
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
    if (p4 == 79 && p5 == 79 && p6 == 79) {
        endg = 1; 
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
    if (p7 == 79 && p8 == 79 && p9 == 79) {
        endg = 1; 
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }

    //Cross
    if (p1 == 79 && p5 == 79 && p9 == 79) {
        endg = 1; 
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
    if (p3 == 79 && p5 == 79 && p7 == 79) {
        endg = 1; 
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "AI Won!!!";
    }
}

int checkalltie(char p1,char p2,char p3,char p4,char p5,char p6,char p7,char p8,char p9){
    if(p1 != 32 && p2 != 32 && p3 != 32 && p4 != 32 && p5 != 32 && p6 != 32 && p7 != 32 && p8 != 32 && p9 != 32 ) {
        endg = 1; 
        system("cls");
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout<<"TIE!!!";}
}

int numberpick() {
    if (l == 'A') {
        switch (n) {
        case 1:
            p1 = 88;
            break;
        case 2:
            p2 = 88;
            break;
        case 3:
            p3 = 88;
            break;
        }
    }
    if (l == 'B') {
        switch (n) {
        case 1:
            p4 = 88;
            break;
        case 2:
            p5 = 88;
            break;
        case 3:
            p6 = 88;
            break;
        }
    }
    if (l == 'C') {
        switch (n) {
        case 1:
            p7 = 88;
            break;
        case 2:
            p8 = 88;
            break;
        case 3:
            p9 = 88;
        }
    }
}

int main() {
    ifendg1:
    checkalltie(p1,p2,p3,p4,p5,p6,p7,p8,p9);

    while (endg == 0) {
        srand(time(0));
        xno(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        cout << "Enter the position: ";
        cin >> l >> n;

        switch (l) {
        case 'A':
            numberpick();
        case 'B':
            numberpick();
        case 'C':
            numberpick();
        }

        checkalltie(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        triplecheckx();
        if(endg==1) goto ifendg1;

        repeat:

        again = 0;
        aivall = (rand() % 3) + 1;
        aivaln = (rand() % 3) + 1;
        ai(aivall, aivaln);
        triplechecko();
        if(endg==1) goto ifendg1;
        if (again == 1) {
            goto repeat;
        }
        checkalltie(p1,p2,p3,p4,p5,p6,p7,p8,p9);
        system("cls");
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Use 'X' and 'O' instead of writing 88 and 79-- similar to how you initialized your values as ' ' and not 32 for clarity e.g. if (p1 == 'X' && p4 == 'X' && p7 == 'X'). Also, many of your variables are declared in global scope, so when you call checkalltie(p1,p2,p3,p4,p5,p6,p7,p8,p9), you are passing variables to the function that are already in the scope of the function (because they are global). It is good practice to not declare global variables. For example, you could use an array char p[9] and then pass the pointer p into the functions that need it e.g. checkalltie(p). \$\endgroup\$ – Kyy13 Oct 16 at 19:06
  • 1
    \$\begingroup\$ @Kyy13 Yea...I guess I could... didnt think about it though when I did this since I'm still fresh (learning from school). But yea good idea. Might actually use it for my next one! Thx. Also I had an issue where the variables wouldnt actually change(if i said for ex x=4 in a function it wont change ouside of it. dont know why) and so I just declared them globaly and left it like that. \$\endgroup\$ – OneDarkGamer Oct 16 at 21:22
  • \$\begingroup\$ The values don't change, because the value is copied into the function. In C++ you can pass a reference instead of a value by adding an & sign after the type i.e. foo(int& a) { a = 3; } will allow you to update the value that is passed into foo. You can also achieve this by providing the address of the variable as a pointer, and then assigning the value by dereferencing it i.e. foo (int* a) { *a = 3; }. Arrays are just pointers to the first element in the array, so you can just use the array name and it will behave like the pointer method. \$\endgroup\$ – Kyy13 Oct 16 at 21:27
1
\$\begingroup\$

The thing I would focus on to start with is reducing code duplication. If you look at your triplechecko/x functions, it's basically the same code over and over but with different variables.

I would start by replacing all the p1...9 with a container. Since it's fixed size, let's use std::array. Then lets see what we can do to reduce some of the code duplication.

#include <array>
#include <ctime>
#include <iostream>

std::array<std::array<char, 3>, 3> board;

bool make_move(char player, char& square) {
    if (square == ' ') {
        square = player;
        return true;
    }
    return false;
}

It's good to give variables and function descriptive names. It might feel a bit tedious when you start out, but you increase the readability a lot. The goal is that you should be able to come back to this code in a few months and have no problem understanding what is what. Names like ai might feel intuitive now, but probably not later on.

ai function became a generic make_move. It does almost the same, but we take a reference to the square we want and the player value 'X' or 'O'. It also returns true or false depending on success. This help us remove global variables like endg and similar.

void print_board() {
    std::cout << "    1  2  3 \n\n";
    char row_char = 'A';
    for (auto& row : board) {
        std::cout << row_char << ". ";
        ++row_char;
        for (auto& square : row) {
            std::cout << square << " ";
        }
        std::cout << "\n\n";
    }

    std::cout << std::endl;
}

Print the board. Using loops to loop through our 2D-array.

bool triple_check(char player, char v1, char v2, char v3) {
    if (v1 == player && v2 == player && v3 == player) {
        std::cout << player << " won!" << std::endl;
        return true;
    }
    return false;
}

bool check_if_won(char player) {
    for (int i = 0; i < 3; ++i) {
        // check rows
        if (triple_check(player, board[i][0], board[i][1], board[i][2])) return true;
        // check columns
        if (triple_check(player, board[0][i], board[1][i], board[2][i])) return true;
    }

    // check diagonals
    if (triple_check(player, board[0][0], board[1][1], board[2][2])) return true;
    if (triple_check(player, board[0][2], board[1][1], board[2][0])) return true;

    return false;
}

Here I decided to split the function in two steps to make it more clear and easy to read. This replaces both triplecheck functions from before, and prints out if someone won. Again we are returning true or false to indicate if someone won.

bool check_tie() {
    for (auto& row : board) {
        for (auto& square : row) {
            if (square == ' ') return false;
        }
    }

    std::cout << "TIE!!!" << std::endl;
    return true;
}

Checking for a tie by looping through the board, return true/false.

bool is_valid_input(char val) {
    if (val >= 0 && val <= 2) return true;
    return false;
}

void number_pick() {
    char row, column;
    while (true) {
        std::cin >> row >> column;

        row -= 'A';
        column -= '1';

        if (is_valid_input(row) && is_valid_input(column) && make_move('X', board[row][column])) return;

        std::cout << "Invalid input. Try again: ";
    }
}

Here we have the function that takes the input from the player. To make the code more clear it's again split in two where we validate the input. I also use row -= 'A' to make the value usable to index into the board array. Additionally we also check the return value of make_move to make sure the move is valid.

void reset_board() {
    for (auto& row : board) {
        for (auto& square : row) {
            square = ' ';
        }
    }
}

int main() {
    srand(time(0));
    reset_board();

    while (true) {
        print_board();

        std::cout << "Enter the position: ";
        number_pick();

        if (check_if_won('X') || check_tie()) break;

        while (!make_move('O', board[rand() % 3][rand() % 3]));

        check_if_won('O');
    }
}

reset_board initializes the board to starting state. If we want to add the option of playing several matches in a row this will come in handy again for resetting the board.

while (!make_move('O', board[rand() % 3][rand() % 3])); uses a while loop without a body, shown by the ; after the condition. It just runs the make_move function over and over with random input until it return true. When it returns true the ! in front makes the loop break.

Now we only have 1 global variable left, the board! If we encasulated this in a class it could be a member variable, but for a small thing like this we'll leave it there.

\$\endgroup\$
  • \$\begingroup\$ I was not taught yet about #include <array> so I dont know anything from it but it looks interesting so I'll look into it further. Also is there any replacement for system("cls")? I heard you shouldnt use system commands at all. And I used it alot! \$\endgroup\$ – OneDarkGamer Oct 16 at 21:26
  • \$\begingroup\$ AFAIK there is no portable way to clear the console. It's platform specific. The only real option I can think of is to use an external library like PDCurses. But often you don't need to clear the console at all since all the new content ends up at the bottom anyway where it's visible. \$\endgroup\$ – super Oct 16 at 21:31

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.