Tic Tac Toe algorithm

Question

I made a Tic Tac Toe game, but I think there's a lot of hard-coding and useless variables. Can you check it out and tell me what I can improve on? It works, but it has a couple of bugs.

#include <iostream>
#include <stdlib.h> 
#include<windows.h>
#include<conio.h>
#include<time.h>
#include<iomanip>
using namespace std;
bool check_right_diagonal(char **arr, int ver_s, int hor_s, bool &win_p1, bool &win_p2, int i, bool &func_worked){
    for (i = 0; i < ver_s; i++){
        for (static int k = ver_s-1; k >= i||k<i ; k--){
            if (arr[i][k] == '_'){
                func_worked = false;
                win_p1 = false;
                win_p2 = false;
                break;
            }
            if (arr[i][k] == 'X'){
                if (win_p2){
                    win_p1 = false;
                    win_p2 = false;
                    break;
                }
                win_p1 = true;
                win_p2 = false;
                break;
            }
            else if (arr[i][k] == 'O'){
                if (win_p1){
                    win_p1 = false;
                    win_p2 = false;
                    break;
                }
                win_p2 = true;
                win_p1 = false;
                break;
            }

        }


    }
    if (win_p1){
        cout << "P1__WON!" << endl;
        func_worked = true;
        return -1;
    }
    else if (win_p2){
        cout << "P2_WON!" << endl;
        func_worked = true;
        return -1;
    }

}
bool check_left_diagonal(char **arr, int ver_s, int hor_s, bool &win_p1, bool &win_p2, int i,bool &func_worked){
    for (i = 0; i < ver_s; i++){
        for (static int k = 0; k < i + 1; k++){
            if (arr[i][k] == '_'){
                func_worked = false;
                win_p1 = false;
                win_p2 = false;
                break;
            }
            if (arr[i][k] == 'X'){
                if (win_p2){
                    win_p1 = false;
                    win_p2 = false;
                    break;
                }
                win_p1 = true;
                win_p2 = false;
            }
            else if (arr[i][k] == 'O'){
                if (win_p1){
                    win_p1 = false;
                    win_p2 = false;
                    break;
                }
                win_p2 = true;
                win_p1 = false;
            }

        }


    }
    if (win_p1){
        cout << "P1__WON!" << endl;
        func_worked = true;
        return -1;
    }
    else if (win_p2){
        cout << "P2_WON!" << endl;
        func_worked = true;
        return -1;
    }

}
bool check_verticalANDhorizontal_win(char **arr, int ver_s, int hor_s, char p1_m, char p2_m, bool &win_p1, bool &win_p2, int i, int j,bool &func_worked){

        for (i = 0; i < ver_s; i++){
            for (j = 0; j < hor_s; j++){
                if (arr[i][j] == '_'){
                    func_worked = false;
                    win_p1 = false;
                    win_p2 = false;
                    break;
                }
                if (arr[i][j] == p1_m){
                        if (win_p2 ){
                        func_worked = false;
                        win_p1 = false;
                        win_p2 = false;
                        break;
                    }
                    win_p1 = true;
                    win_p2 = false;
                }
                else if (arr[i][j] == p2_m ){
                    if (arr[i][j] == '_')break;
                    if (win_p1){
                        func_worked = false;
                        win_p1 = false;
                        win_p2 = false;
                        break;
                    }
                    win_p1 = false;
                    win_p2 = true;
                }



            }
            if (win_p1 || win_p2 && j == hor_s)break;
        }
        if (win_p1){

            cout << "P1__WON!" << endl;
            func_worked = true;
            return -1;
        }
        else if (win_p2){
            func_worked = true;
            cout << "P2_WON!" << endl;
            return -1;
        }
        func_worked = false;
        return 0;

}
void draw_field(char **arr,int size){
    for (int i = 0; i < size; i++)
        cout<<setw(4)<< i;
    cout << endl;
    for (int i = 0; i < size; i++){
        cout << i<<setw(3);
        for (int j = 0; j < size; j++){     
            cout << arr[i][j]<<"   ";
        }
        cout << endl;
    }
}

bool check_occupied_pos(int p_i,int p_j,char **arr,char p1_mark,char p2_mark){
    if (arr[p_i][p_j] == p1_mark || arr[p_i][p_j] == p2_mark){
        cout << "Teritorry is occupied"<< endl;
        return false;
    }
    else if (arr[p_i][p_j] == p2_mark || arr[p_i][p_j] == p1_mark){
        cout << "Teritorry is occupied"<< endl;
        return false;
    }
    else
        return true;

}
void TIC_TAC_TOE_PRINT(){
    char mass[9][74] = {
        {"TTTTTTTT  IIII  CCCCCC  TTTTTTT  AAAAAA  CCCCCC  TTTTTTT  OOOOOO  EEEEEEE"},
        {"TTTTTTTT  IIII  CCCCCC  TTTTTTT  AAAAAA  CCCCCC  TTTTTTT  OOOOOO  EEEEEEE" },
        {"  TTT           CC        TTT    AA  AA  CC        TTT    OO  OO  EE     "},
        {"  TTT      II   CC        TTT    AA  AA  CC        TTT    OO  OO  EEEEE  "},
        {"  TTT      II   CC        TTT    AA  AA  CC        TTT    OO  OO  EEEEE  "},
        {"  TTT      II   CC        TTT    AAAAAA  CC        TTT    OO  OO  EE     "},
        {"  TTT      II   CCCCCC    TTT    AA  AA  CCCCCC    TTT    OOOOOO  EEEEEEE"},
        {"  TTT      II   CCCCCC    TTT    AA  AA  CCCCCC    TTT    OOOOOO  EEEEEEE"}
    };
    for (int i = 0; i < 9; i++){
        for (int j = 0; j < 74; j++){
            cout <<mass[i][j];
        }
        cout << endl;
    }

}
void WinGame(char **arr,int ver_s,int hor_s, char p1_m,char p2_m,bool &GAME_START){
    bool win_p1 = false;
    bool win_p2 = false;
    bool func_worked = false;
    int j = 0;
    int i = 0;

    check_verticalANDhorizontal_win(arr, ver_s, hor_s, p1_m, p2_m, win_p1, win_p2, i, j,func_worked);
    if (!func_worked)
    check_verticalANDhorizontal_win(arr, ver_s, hor_s, p1_m, p2_m, win_p1, win_p2, j, i,func_worked);
    if (!func_worked){
    check_left_diagonal(arr, ver_s, hor_s, win_p1, win_p2, i,func_worked);
    }
    if (!func_worked){
        check_right_diagonal(arr, ver_s, hor_s, win_p1, win_p2, i, func_worked);
    }
    if (win_p1 && !win_p2 || win_p1 && win_p2)
        GAME_START = false;
    else
        GAME_START = true;



}
int main()
{
    char p1_mark = 'X'; char p2_mark = 'O';

    HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
    SetConsoleTextAttribute(h, FOREGROUND_GREEN | FOREGROUND_INTENSITY);
    TIC_TAC_TOE_PRINT();
    SetConsoleTextAttribute(h, FOREGROUND_BLUE | FOREGROUND_INTENSITY); 
    //Initialization-------------------------------
    int ver_s =  3;
    int hor_s = 3;
    bool GAME_START = TRUE;
    int field_size = ver_s*hor_s;
    int *fields = new int[field_size];
    for (int i = 0; i < field_size; i++){
        fields[i] = 0;
        fields[i] = fields[i] + i;

    }

    char **GameField = new char*[ver_s];
    for (int i = 0; i < ver_s; ++i)
        GameField[i] = new char[hor_s];

    for (int i = 0; i < ver_s; i++){
        for (int j = 0; j < hor_s; j++){
            GameField[i][j] ='_' ;

        }


    }
    draw_field(GameField, ver_s);

    //end of initialization-------------------------Beginning of tic tac toe game logic
    //int P1_random_num = 4;
    //int P2_random_num = 3;
    bool p2_try = false;
    bool p1_try = true;
    int p1_move_choice_i = 0;
    int p1_move_choice_j = 0;

    int p2_move_choice_i = 0;
    int p2_move_choice_j = 0;
        while (GAME_START){
            if (p1_try){
                cout << "Player_1,enter two numbers(position) of where to place" << endl;
                cin >> p1_move_choice_i;
                cin >> p1_move_choice_j;
                if (check_occupied_pos(p1_move_choice_i, p1_move_choice_j, GameField, p1_mark, p2_mark)){
                    GameField[p1_move_choice_i][p1_move_choice_j] = NULL;
                    GameField[p1_move_choice_i][p1_move_choice_j] = 'X';
                    draw_field(GameField, ver_s);
                    p1_try = false;
                    p2_try = true;
                }
                else continue;
            }

            else if (p2_try){
                cout << "Player_2, enter two numbers(position) of where to place" << endl;
                cin >> p2_move_choice_i;
                cin >> p2_move_choice_j;
                if (check_occupied_pos(p2_move_choice_i, p2_move_choice_j, GameField, p1_mark, p2_mark)){
                    GameField[p2_move_choice_i][p2_move_choice_j] = NULL;
                    GameField[p2_move_choice_i][p2_move_choice_j] = 'O';
                    draw_field(GameField, ver_s);
                    p2_try = false;
                    p1_try = true;
                }
                else continue;
            }
            WinGame(GameField, ver_s, hor_s, p1_mark, p2_mark, GAME_START);


        }



}

Answer 1

using namespace std;

This should be avoided. The reasons are explained in detail in that question and its answers on Stack Overflow. Instead you should add the namespace in front of every function call from that namespace, e. g. std::cout instead of cout. The additional typing is very little and the better readabilty (and avoiding further subtle problems) is worth it.

Naming

You named two of your functions check_right_diagonal and check_left_diagonal, however from reading the name it is still not obvious which diagonal is refered to, because how can a diagonal be on any side? You might differentiate the diagonals by calling them "upwards" and "downwards", but even that is only slightly better and still not precise.

Instead I recommend to just make one function check_diagonals, and check both in the same function. We will get to that later when we talk about the algorithms.

---

check_verticalANDhorizontal_win is very unconventional, and unconventional is usually bad. It is important to be consistent with your naming scheme, otherwise it will be difficult to read the code.

The convention would be to name it check_vertical_and_horizontal_win. But now it is only consistent in style, still not logically. You didn't name the other functions check_diagonal_win, so I suggest to leave out the "win" here as well. After all it is pretty clear what is checked for in tic-tac-toe. Furthermore, a method should only do one thing, and method names that contain "and" are often a sign that it should be split up into two methods, here check_horizontal and check_vertical, or better and shorter check_rows and check_columns.

Note that these names might still need to be changed if it turns out that we find a better way to do the algorithms.

---

TIC_TAC_TOE_PRINT is another atypical naming scheme, and the functions should rather be called something like print_tic_tac_toe_logo. The name you chose (aside from the capitalization) sounds more like a custom version of any print function, like for example printf.

---

WinGame

Yet another capitalization style. This should be win_game. Probably it should even be something like is_game_over or has_won, or something similar. But then it should be returning a boolean.

---

You are using a lot of parameters and variables, so I won't list all of them, but basically all should have better speaking names, that tell you what they contain. In C++ names are often kept short, but for example your arr is actually your board. The others are very cryptic, but we probably won't even need most of them. We will get to that.

Logic and Bugs

There seems to be a bug, or maybe just a redundancy, in the function WinGame.

if (win_p1 && !win_p2 || win_p1 && win_p2)

It seems like you want to check "has player 1 won but not player 2, or vice versa". Instead you check in the second part whether any of the two players has won, so the entire expression is equivalent to just (win_p1 && win_p2), assuming that you don't have to explicitly check whether both players have won, which shouldn't be possible anyway.

---

When checking whether a player has won, you do this:

check_verticalANDhorizontal_win(arr, ... , func_worked);
if (!func_worked)

I shortened the call to hide some of the parameters. You are passing the variable by reference and changing its value, which can easily lead to hard to find bugs, especially when there is an easier and more intuitive way of doing it. Furthermore you have different variables for both players whether they have won or not, and still the function is returning boolean whose value always seems to be -1.

If you changed your function to return the number of the player (1 or 2) if the player has won, and return 0 if none of them has won, you wouldn't need most of the parameters. Even better, if you only check whether the current player has won after he placed his symbol on the board, then you could just write the check in WinGame like this:

if (is_vertical_win(...) || is_horizontal_win(...) || is_diagonal_win(...))

(Note that again I omitted the parameters just for the exmaple.) Doing it that way you can also avoid setting a variable named func_worked, which is misleading because no matter whether the result is true or false, in both cases the function has worked. Just the result of the check would be different.

Algorithms and Data Structures

You are checking the board as if it could have a dynamic size. But since you hardcoded the size to 3x3, and it is always 3x3 in tic-tac-toe, you can just do some simple checks instead of these 3 very long functions with nested loops.

This is the vertical check (board is your arr, since field fits better as a name):

if (board[0][0] == board[0][1] && board[0][0] == board[0][2]
 || board[1][0] == board[1][1] && board[1][0] == board[0][2]
 || board[2][0] == board[2][1] && board[2][0] == board[0][2])
{
    return true;
}

This is the horizontal check:

if (board[0][0] == board[1][0] && board[0][0] == board[2][0]
 || board[0][1] == board[1][1] && board[0][1] == board[2][1]
 || board[0][2] == board[1][2] && board[0][2] == board[2][2])
{
    return true;
}

This is the diagonal check:

if (board[0][0] == board[1][1] && board[0][0] == board[2][2]
 || board[0][2] == board[1][1] && board[0][2] == board[2][0])
{
    return true;
}

In any other case we just return false:

return false;

Here we are only checking whether they are equal, not which symbol they contain. But we do not have to do that if we only check for a win after any symbol is placed, and we store the current symbol somewhere.

---

You are using many C constructs, while you are writing your program in C++. But even though most C code is valid in C++, there is often a simpler or better way to do it in C++. For example, you could replace your array of pointers, which you use for the board, with a std::vector of std::vectors. That way you don't need as much pointer arithmetic (which is error prone) and also you don't have to pass the array size every time you access the board.