I decided to make a simple tic-tac-toe game in C++ as a small project. All help and suggestions are greatly appreciated!

main.cpp:

#include <iostream>
#include <string>
#include "TicTacToe.hpp"

TicTacToe game;

void turnPlayer1();
void turnPlayer2();

bool gameOver = false;

int main() {

std::cout << "*********************************" << std::endl;
std::cout << "Hello and welcome to tic-tac-toe!" << std::endl;
std::cout << "*********************************" << std::endl;
std::cout << "\n\n\n\n";

game.initBoard(); 
game.printBoard();

while(gameOver == false) {
    turnPlayer1();
    if(gameOver == true) {
        break;
    }
    turnPlayer2();
}

return 0;
}

void turnPlayer1() {
bool madeTurn = false;

while(madeTurn != true) {
    int x;
    int y;

    std::cout << "Which row do you choose player 1?" << std::endl;
    std::cin >> x;
    std::cout << "Which column do you choose player 1?" << std::endl;
    std::cin >> y;

    if(game.checkPlace(x, y)) {

        game.placePiecePlayer1(x, y);
        game.printBoard();

        if(game.checkVictoryPlayer1()) {
            std::cout << "Congrats! Player 1 has won!" << std::endl;
            gameOver = true;
        }

        madeTurn = true;


    } else {
        std::cout << "Oh no! Those coordinates are not valid! Please try again." << std::endl;
        madeTurn = false; 
    }

}
}

void turnPlayer2() {
bool madeTurn = false;

while(madeTurn != true) {
    int x;
    int y;

    std::cout << "Which row do you choose player 2?" << std::endl;
    std::cin >> x;
    std::cout << "Which column do you choose player 2?" << std::endl;
    std::cin >> y;

    if(game.checkPlace(x, y)) {

        game.placePiecePlayer2(x, y);
        game.printBoard();

        std::cout << game.checkVictoryPlayer2() << std::endl;

        if(game.checkVictoryPlayer2()) {
            std::cout << "Congrats! Player 2 has won!" << std::endl;
            gameOver = true;
        }

        madeTurn = true;


    } else {
        std::cout << "Oh no! Those coordinates are not valid! Please try again." << std::endl;
        madeTurn = false;
    }

}
}

TicTacToe.hpp:

#ifndef TicTacToe_hpp
#define TicTacToe_hpp

#include <iostream>
#include <stdio.h>
#include <string>

class TicTacToe {
public:
TicTacToe(); 
void printBoard();
void placePiecePlayer1(int x, int y);
void placePiecePlayer2(int x, int y);
void initBoard(); 
bool checkPlace(int x, int y);
bool checkVictoryPlayer1();
bool checkVictoryPlayer2(); 
private:
std::string _gameBoard[4][4];
std::string _player1Piece = "[x]";
std::string _player2Piece = "[o]";
std::string _emptySpace = "[ ]";
};

#endif /* TicTacToe_hpp */

TicTacToe.cpp:

#include "TicTacToe.hpp"

TicTacToe::TicTacToe() {}

void TicTacToe::initBoard() {
for(int y = 0; y < 4; y++) {
    for(int x = 0; x < 4; x++) {
        if(x == 0 && y == 0) {
            _gameBoard[0][0] = " 0 ";
        } else if((x == 1 && y == 0) || (x == 0 && y == 1)) {
            _gameBoard[y][x] = " 1 ";
        } else if((x == 2 && y == 0) || (x == 0 && y == 2)) {
            _gameBoard[y][x] = " 2 ";
        } else if((x == 3 && y == 0) || (x == 0 && y == 3)) {
            _gameBoard[y][x] = " 3 ";
        } else {
            _gameBoard[y][x] = _emptySpace;
        }
      }
    }
 }

void TicTacToe::printBoard() {
for(int y = 0; y < 4; y++) {
    for(int x = 0; x < 4; x++) {
        std::cout << _gameBoard[y][x];
    }
    std::cout << std::endl;
}
}

void TicTacToe::placePiecePlayer1(int x, int y) {
_gameBoard[y][x] = _player1Piece;
}

void TicTacToe::placePiecePlayer2(int x, int y) {
_gameBoard[y][x] = _player2Piece;
}

bool TicTacToe::checkPlace(int x, int y) {
bool placeOpen = true;
if(_gameBoard[y][x] != _emptySpace) {
    placeOpen = false;
} else if(_gameBoard[y][x] == _emptySpace) {
    placeOpen = true;
}
return placeOpen;
}

bool TicTacToe::checkVictoryPlayer1() {
bool hasWon;
if((_gameBoard[1][1] == _player1Piece) && (_gameBoard[1][2] ==    _player1Piece) && (_gameBoard[1][3] == _player1Piece)) {
    hasWon = true; // top horizontal
} else if((_gameBoard[2][1] == _player1Piece) && (_gameBoard[2][2] == _player1Piece) && (_gameBoard[2][3] == _player1Piece)) {
    hasWon = true; // middle horizontal
} else if((_gameBoard[3][1] == _player1Piece) && (_gameBoard[3][2] == _player1Piece) && (_gameBoard[3][2] == _player1Piece)) {
    hasWon = true; // bottom horizontal
} else if((_gameBoard[1][1] == _player1Piece) && (_gameBoard[2][1] == _player1Piece) && (_gameBoard[3][1] == _player1Piece)) {
    hasWon = true; // left vertical
} else if((_gameBoard[1][2] == _player1Piece) && (_gameBoard[2][2] == _player1Piece) && (_gameBoard[3][2] == _player1Piece)) {
    hasWon = true; // middle vertical
} else if((_gameBoard[1][3] == _player1Piece) && (_gameBoard[2][3] == _player1Piece) && (_gameBoard[3][3] == _player1Piece)) {
    hasWon = true; // right vertical
} else if((_gameBoard[1][1] == _player1Piece) && (_gameBoard[2][2] == _player1Piece) && (_gameBoard[3][3] == _player1Piece)) {
    hasWon = true; // top left to bottom right
} else if((_gameBoard[1][3] == _player1Piece) && (_gameBoard[2][2] == _player1Piece) && (_gameBoard[1][3] == _player1Piece)) {
    hasWon = true; // top right to bottom left
} else {
    hasWon = false;
}
return hasWon;
}

bool TicTacToe::checkVictoryPlayer2() {
bool hasWon;
if((_gameBoard[1][1] == _player2Piece) && (_gameBoard[1][2] == _player2Piece) && (_gameBoard[1][3] == _player2Piece)) {
    hasWon = true; // top horizontal
} else if((_gameBoard[2][1] == _player2Piece) && (_gameBoard[2][2] == _player2Piece) && (_gameBoard[2][3] == _player2Piece)) {
    hasWon = true; // middle horizontal
} else if((_gameBoard[3][1] == _player2Piece) && (_gameBoard[3][2] == _player2Piece) && (_gameBoard[3][2] == _player2Piece)) {
    hasWon = true; // bottom horizontal
} else if((_gameBoard[1][1] == _player2Piece) && (_gameBoard[2][1] == _player2Piece) && (_gameBoard[3][1] == _player2Piece)) {
    hasWon = true; // left vertical
} else if((_gameBoard[1][2] == _player2Piece) && (_gameBoard[2][2] == _player2Piece) && (_gameBoard[3][2] == _player2Piece)) {
    hasWon = true; // middle vertical
} else if((_gameBoard[1][3] == _player2Piece) && (_gameBoard[2][3] == _player2Piece) && (_gameBoard[3][3] == _player2Piece)) {
    hasWon = true; // right vertical
} else if((_gameBoard[1][1] == _player2Piece) && (_gameBoard[2][2] == _player2Piece) && (_gameBoard[3][3] == _player2Piece)) {
    hasWon = true; // top left to bottom right
} else if((_gameBoard[1][3] == _player2Piece) && (_gameBoard[2][2] == _player2Piece) && (_gameBoard[1][3] == _player2Piece)) {
    hasWon = true; // top right to bottom left
} else {
    hasWon = false;
}
return hasWon;
}

There's really a lot going on in your code; I'll never be able to address them all, so I'll just address a few of the major things.

In TicTacToe.cpp you have the routine checkVictoryPlayer1() and then, later on, you have a nearly identical routine called checkVictoryPlayer2().

Imagine for a moment, what you would need to do if you invented a new game called "TicTacToePlusPlus", and it supported 3 players.

As written here, you'd need to add a new routine checkVictoryPlayer3(). And once it became popular, you'd need to add support for four players!

I think you can see the beginnings of the problems.

In general, if you find yourself repeating large sections of code, and just changing one or two things about it, you should start thinking about rewriting it, and pass in the part that changes as a parameter.

So, I'd suggest changing:

bool TicTacToe::checkVictoryPlayer1() {
    bool hasWon;

    if ((_gameBoard[1][1] == _player1Piece) 
        && (_gameBoard[1][2] == _player1Piece)
       && (_gameBoard[1][3] == _player1Piece)) 
    {
        hasWon = true; // top horizontal

       ----8<----- SNIPPED OUT A LOT OF CODE ----8<-----

to a routine that accepts the player as a parameter:

bool TicTacToe::checkVictory( std::string aPlayer ) {
    bool hasWon;

    if    ((_gameBoard[1][1] == aPlayer ) 
        && (_gameBoard[1][2] == aPlayer )
        && (_gameBoard[1][3] == aPlayer )) 
    {
        hasWon = true; // top horizontal

   //  etcetera...

This is a big deal.

Software engineers call this "code reuse", and it's one of our major goals. Less code means less work debugging, and less time writing the code. It also makes it easier to verify and validate that the code does exactly what we think it's doing. Our confidence in the "correctness" of the solution goes up.

Along this same line, though, you can still see that equivalent statements are duplicated again and again in checkVictory:

if ((_gameBoard[1][1] == _player1Piece) 
   && (_gameBoard[1][2] == _player1Piece) 
   && (_gameBoard[1][3] == _player1Piece)) 
{
    hasWon = true; // top horizontal
}
else if ((_gameBoard[2][1] == _player1Piece) 
     && (_gameBoard[2][2] == _player1Piece) 
     && (_gameBoard[2][3] == _player1Piece)) 
     {
        hasWon = true; // middle horizontal
    }
else if ((_gameBoard[3][1] == _player1Piece) 
     && (_gameBoard[3][2] == _player1Piece) 
     && (_gameBoard[3][2] == _player1Piece)) 
     {
        hasWon = true; // bottom horizontal
    }

Notice how the only thing changing in each of these statements is the first index to the array.

This means we can, again, turn it into a parameter to a function that can work in a general way, for the horizontal rows.

Something like this:

bool TicTacToe::checkHorizontalRow( int rowNumber, std::string aPlayer )
{
     bool isAWinner = false;   // Assume they are not a winner!

     if (   (_gameBoard[ rowNumber ][1] == aPlayer ) 
         && (_gameBoard[ rowNumber ][2] == aPlayer ) 
         && (_gameBoard[ rowNumber ][3] == aPlayer )) 
     {
        isAWinner = true; // top horizontal
     }
     return isAWinner;
}

Once we create that routine, we can use it as follows:

bool TicTacToe::checkVictory( std::string aPlayer ) {
    bool hasWon;

    hasWon = (  checkHorizontalRow( 1, aPlayer )
             || checkHorizontalRow( 2, aPlayer )
             || checkHorizontalRow( 3, aPlayer ) );

This checks all three horizontal rows for a player.

Now you should try and create the routine that does the Vertical Rows, and include it in the checkVictory subroutine.

How would you do it? Give it a try, and post it.

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.