HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Making this Tic Tac Toe implementation more recursive

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thistoeticmoretacrecursiveimplementationmaking

Problem

Here is a Tic Tac Toe game in C++ that I have been working on. My questions are as follows:

-
I tested it for bugs, but if you find any, please post the solution!

-
I want to make the code more recursive, in a sense. Instead of having multiple lines of code that do the same things with different values, it would be nice to compress it even more. You see, I'm a minimalist and I'm lazy (fundamental programmer traits!) so less typing is good.

-
I'm wondering if my code follows the best common practices, and what those practices might be. For instance, is using camelCase for variables and something_like_this good for functions?

-
If you have any other comments on the code, please let me know!

main.cpp

```
/*Tic Tac Toe Game v1.0
Released under the GPL liscence version 2
Written by JohnBobSmith*/

#include
#include //This is required to catch invalid user input

class tic_tac_toe //A class to contain all our functions
{
public: //Allow us to use the functions anywhere
char board[3][3]; //Creates an 2d array for the board
char previousPlayerPiece; //variable used in multiple functions
char previousPlayer2Piece; //varibable used in multiple functions

void init_board()
{
//initializes a blank board
for(int a = 0; a > x;
if (x 3 || std::cin.fail()){
std::cin.clear();
std::cin.ignore(std::numeric_limits::max(),'\n');
std::cout > y;
if (y 3 || std::cin.fail()){
std::cin.clear();
std::cin.ignore(std::numeric_limits::max(),'\n');
std::cout << "Your number is invalid. Try again. " << std::endl;
} else {
break;
}
}

if (check_for_overlap(x, y, previousPlayerPiece) == true){
x--;y--;print_board(previousPlayer2Piece, x, y);
std::cout << "\nThe board has been re-set. Try again!" << std::endl;
} else if (check_for_overlap(x

Solution

The most obvious improvement will come from the check_for_overlap function. You can replace your entire implementation with:

int check_for_overlap(int pos1, int pos2, char playerPiece)
{
    if (board[pos1-1][pos2-1] != '-'){
        std::cout << "\nOVERLAP DETECTED!!!!!!" << std::endl;
        return true;
    }
    return false;
}


The next thing I noticed was the difference between the user interface (1-based coordinates) and the internal representation (0-based coordinates). This is fine, but your implementation is a bit awkward in this area. You have lines like

x--;y--;print_board(previousPlayer2Piece, x, y);


which is a pretty unusual style. An improvement would be:

print_board(previousPlayer2Piece, x-1, y-1);


A better implementation might be to get the user input into different variables, such as userRow and userCol. So you would have (omitting your input validation and retry logic):

int userRow, userCol;
std::cin >> userRow;
std::cin >> userCol;
int x = userRow - 1;
int y = userCol - 1;


The == true check is redundant in a couple of places, since your functions already return bool. So instead of

if (check_for_overlap(x, y, previousPlayerPiece) == true){


use

if (check_for_overlap(x, y, previousPlayerPiece)){


In check_for_win, you can use loops to reduce the amount of code. For example:

for (int i = 0; i < 3; i++) {
        if (board[i][0] == playerPiece && board[i][1] == playerPiece && board[i][2] == playerPiece){
            std::cout << "\nPlayer " << playerPiece << " wins!" << std::endl;
            return true;
        }
    }


You can use similar loops to check the column wins.

Code Snippets

int check_for_overlap(int pos1, int pos2, char playerPiece)
{
    if (board[pos1-1][pos2-1] != '-'){
        std::cout << "\nOVERLAP DETECTED!!!!!!" << std::endl;
        return true;
    }
    return false;
}
x--;y--;print_board(previousPlayer2Piece, x, y);
print_board(previousPlayer2Piece, x-1, y-1);
int userRow, userCol;
std::cin >> userRow;
std::cin >> userCol;
int x = userRow - 1;
int y = userCol - 1;
if (check_for_overlap(x, y, previousPlayerPiece) == true){

Context

StackExchange Code Review Q#55074, answer score: 6

Revisions (0)

No revisions yet.