My tic tac toe minimax implementation

Question

This is my first time using StackExchange. I did not have any idea what this website for, but I guess is a place to review the code? So, I'm just going to give it a shot.

#include <iostream>

std::string board[3][3];

void making_board();
void play_the_game();
void display_board();
void choose_position(int position, std::string* current_player);
void find_row_column(int position, int* row, int* column);
void player_turn(std::string* current_player);
std::string computer_turn(std::string* current_player);
std::string predict_future(std::string* current_player);
std::string check_the_game();
int possible_move(int* row, int* column, std::string *current_player, std::string* position);
int move_left(int row, int column, std::string* current_player);
int predict_opponent(int* row, int* col, std::string opponent, std::string* ori_position);
int possible_opponent(int* row, int* column, std::string opponent);

int main()
{
    play_the_game();
    return 0;
}

std::string check_the_game(){
    int area_filled = 0;
    for (int row = 0; row < 3; row++){
        if (board[row][0] == board[row][1] && board[row][1] == board[row][2]){
            return board[row][0];
        }
    }
    for (int column = 0; column < 3; column++){
        if (board[0][column] == board[1][column] && board[1][column] == board[2][column]){
            return board[0][column];
        }
    }
    if (board[0][0] == board[1][1] && board[1][1] == board[2][2])return board[1][1];
    else if (board[2][0] == board[1][1] && board[1][1] == board [0][2]) return board[1][1];
    for (int rows = 0; rows < 3; rows++){
        for (int columns = 0; columns < 3; columns++){
            if (board[rows][columns] == "X" || board[rows][columns] == "O")
            area_filled++;
            if (area_filled == 9)return "neither";
        }
    }
    return "notyet";
}

int possible_opponent(int row, int column, std::string opponent){
    std::string ori_position = board[row][column];
    board[row][column] = opponent;
    if (check_the_game() == "X"){
        board[row][column] = ori_position;
        return -10;
    }else {
        board[row][column] = ori_position;
        return 0;
    }
}

int predict_opponent(int* row, int* col, std::string opponent, std::string* ori_position){
    int score;
    for (int r = 0; r < 3; r++){
        for (int c = 0; c < 3; c++){
            if (board[r][c] != "X" && board[r][c] != "O"){
                if (possible_opponent(r, c, opponent) == -10){
                    board[*row][*col] = *ori_position;
                    *row = r;
                    *col = c;
                    return -10;
                } else continue;
            }else continue;
        }
    }
    return 0;
}

int possible_move(int* row, int* column, std::string *current_player, std::string* position){
    std::string ori_position = board[*row][*column];
    board[*row][*column] = *current_player;
    if (check_the_game() == "O"){
        board[*row][*column] = ori_position;
        return 10;
    }else if (predict_opponent(row, column, "X", &ori_position) == -10){
        *position = board[*row][*column];
        board[*row][*column] = "O";
        return -10;
    }else {
        board[*row][*column] = ori_position;
        return 0;
        }
}

std::string predict_future(std::string* current_player){
    std::string position;
    int score;
    for (int row = 0; row < 3; row++){
        for (int column = 0; column < 3; column++){
            if (board[row][column] != "X" && board[row][column] != "O"){
                if (possible_move(&row, &column, current_player, &position) == -10){
                    return position;
                }
                else if (possible_move(&row, &column, current_player, &position) == 10){
                    position = board[row][column];
                    board[row][column] = *current_player;
                    return position;
                }else if (move_left(row, column, current_player) == 1){
                    position = board[row][column];
                    board[row][column] = *current_player;
                    return position;
                }else if(possible_move(&row, &column, current_player, &position) == 0){
                    if (board[1][1] != "X" && board[1][1] != "O" && move_left(row, column, current_player) < 9){
                        position = board[1][1];
                        board[1][1] = *current_player;
                        return position;
                    }else continue;
                }
            }else continue;
        }
    }
}

int move_left(int row, int column, std::string* current_player){
    int move_left = 0;
    for (int r = row; r < 3; r++){
        for (int c = column; c < 3; c++){
            if (board[r][c] != "X" && board[r][c] != "O"){
                move_left++;
            }
        }
    }
    return move_left;
}

void play_the_game(){
    bool still_playing = true;
    std::string current_player = "X";
    std::string winner = "";
    making_board();
    while (still_playing){
        display_board();
        player_turn(&current_player);
        winner = check_the_game();
        if (winner == "X" || winner == "O" || winner == "neither"){display_board();break;}
    }
    std::cout << " Winner is " << winner << std::endl;
    std::cout << " Play again (1 for yes and 0 for no)? ";
    std::string answer;
    std::cin >> answer;
    if (answer == "1"){
        system("CLS");
        play_the_game();
    }else if(answer == "0"){
        exit(0);
    }else {
        std::cout << " Invalid answer";
        std::cout << " Play again (1 for yes and 0 for no)? ";
    }
}

void player_turn(std::string *current_player){
    std::string sposition;
    int position;
    if (*current_player == "O"){
        sposition = predict_future(current_player);
        std::cout << " " << *current_player << "'s turn, position chosen was: " << sposition << std::endl;
        *current_player = "X";
    }else {
        std::cout << " " << *current_player << "'s turn, choose position: ";
        std::cin >> position;
        if (!std::cin >> position){
            std::cin.clear();
            std::cin.ignore(1000, '\n');
            std::cout << " Position is not available" << std::endl;
            player_turn(current_player);
        }
        choose_position(position, current_player);
    }
}

void choose_position(int position, std::string *current_player){
    int row, column;
    find_row_column(position, &row, &column);
    if (board[row][column] == "X" || board[row][column] == "O" || position < 0 || position > 9){
        std::cout << " Position is not available" << std::endl;
        player_turn(current_player);
    } else {
        board[row][column] = *current_player;
        if (*current_player == "X") *current_player = "O";
        else *current_player ="X";
    }
}

void find_row_column(int position, int* row, int* column){
    *row = position/3;
    if (*row = 0) *column = 2;
    *column = position-*row-1;
}

void display_board(){
    std::cout << "\n";
    for (int rows = 2; rows >= 0; rows--){
        for (int columns = 0; columns < 3; columns++){
            if (columns == 0){
                std::cout << "  " << board[rows][columns];
            } else std::cout << " | " << board[rows][columns];
        }
        if (rows > 0)std::cout << "\n ---|---|---\n";
        else std::cout << "\n\n";
    }
}

void making_board(){
    int i = 1;
    for (int rows = 0; rows < 3; rows++){
        for (int columns = 0; columns < 3; columns++){
            board[rows][columns] = std::to_string(i);
            i++;
        }
    }
}

I tried to beat my AI. Somehow I still won.

Answer 1

Code structure review

1. It is C-style code. As you have added C++ tag, I assume you intend to write C++ code. Think from OOP point of view and abstractions will improve. Methods and signatures will improve and unnecessary passing of parameters will be avoided.

void choose_position(int position, std::string* current_player);
void find_row_column(int position, int* row, int* column);
void player_turn(std::string* current_player);
std::string computer_turn(std::string* current_player);
std::string predict_future(std::string* current_player);
std::string check_the_game();
int possible_move(int* row, int* column, std::string* current_player, std::string* position);
int move_left(int row, int column, std::string* current_player);
int predict_opponent(int* row, int* col, std::string opponent, std::string* ori_position);
int possible_opponent(int* row, int* column, std::string opponent);

2. Board representation: Using 2D string array to represent board is not right choice. It is making code unnecessarily complex. Boolean array will be a better choice. For example function check_the_game is doing lots of string comparisons. With boolean array, it can be done using few AND operations. Return value of this function can be a int (or enum).

3. Model and view should be separated. It will also allow you to bind any shape you want to use to represent icons for two player.

4. Refs vs pointers: Code can be written with refs rather than pointers.

5. Strings "X" and "O" is sprinkled throughout the code. It should be avoided.

6. Random constants like -10, 10, 9, 1000 etc sprinkled throughout the code. It should be avoided.

Summary: Rethink from OOP perspective. Separate model and view. Avoid random constants.

Answer 2

No time for a full review, but here are some general remarks:

• making_board: try and use verbs, as methods are things that perform actions, i.e. make_board or setup_board (you seem to do this right for the other methods, so maybe this is just the odd one out).
• C++ code often differs from C by using capital letters instead of underscores to separate method names. Style is subjective though - as long as you are aware of this.
• Use explicit WIDTH and HEIGHT instead of literals valued 2, 3 and 9.
• check_the_game should have separate methods to check for rows, columns and diagonals. You could also explicitly define "groups", which could be either, but this is a more advanced trick (I use it for my SudokuSolver code, where a Sudoku can have many forms).
• You could use an enum with PLAYING, DRAW, WIN_O and WIN_X constants, try and avoid "stringly typed" code.
• Even if you don't split your code in separate classes, it makes sense to order them somewhat, e.g. first main, then play_the_game etc.. The declarations and definitions should probably have the same order as well, so people don't have to search when using a text editor (or GitHub raw code, yuck).
• Try and avoid readers to have to guess what e.g. -10 means, a constant value could have been used.
• else continue is completely unnecessary at the end of a code block within a loop - trust me, any loop will continue automatically; it's what they do.
• You could try and make a stream of positions instead of iterating over them all the time (this goes into lambda / functional territory though). This would also make sure that you don't get "X-mas tree code blocks" with many nested for and if statements.
• Closing parentheses should be followed by a space, e.g. ) { instead of ){, maybe put your code though lint or another formatter / static style analyzer.

Answer 3

Use another type for board representation

Maintaining an array of strings, constantly making comparisons and possibly copying it in a few places can get very expensive very fast, which makes representing the board internally using std::string inefficient.

Using a plain char is more suitable, maybe even Enums to map the characters to integral vales.

Use references

Prefer using references over pointers when possible. Aside from looking cleaner syntactically, using a reference makes your intention clear. This SO thread contains further discussion on this topic

More well-defined types

It seems to me that the code quality could largely benefit from defining proper types for a few things.

A very simple example from your code is the way you define positions on the board.

Even the ancient struct Point {int x, y; } with a few helper functions can make the code clearer, as you don't have to rely on passing around raw ints everywhere. The same applies to your board re-presentation. Have a look at object-oriented programming.

Answer 4

int possible_opponent(int row, int column, std::string opponent){

You're passing opponent by value, which duplicates the string. It's normal to pass strings by const reference, or better yet use a string_view.

    std::string ori_position = board[row][column];
    board[row][column] = opponent;
    if (check_the_game() == "X"){
        board[row][column] = ori_position;
        return -10;
    }else {
        board[row][column] = ori_position;
        return 0;
    }
}

So, you make a move, check the result, and then undo the move?
At the very least, it's poor to duplicate the board[row][column] = ori_position; in both branches.

If you make your board using byte-sized enumeration constants or characters rather than string objects for each cell, it becomes practical to copy the entire board state. In fact, a 9-byte board is smaller than a string instance! Making intermediate copies of the state is more normal for this kind of mini-maxi or speculative play algorithm.