1
\$\begingroup\$

I've written a TicTacToe game using classes, just for learning purposes... and I wonder If this code can be improved.

main.cpp

#include <iostream>

#include "tictactoe.hpp"

using namespace std;

int main(){
    Tictactoe game;
    game.intro();
    return 0;
}

tictactoe.hpp

#ifndef tictactoe_hpp
#define tictactoe_hpp
#include <iostream>
#include <stdio.h>

using namespace std;

class Tictactoe {
public:
    // Constructor and Destructor
    Tictactoe();
    ~Tictactoe();

    // Functions
    int placeToken(int, int, int);
    bool checkHorizontal(int);
    bool checkVertical(int);
    bool checkDiagonal(int);
    bool playerWon(int);
    bool positionIsTaken(int, int);
    bool isTie();

    void intro();
    void gamePlay();
    void showBoard();

private:
    // Variables
    int board[3][3], pos[2], turn;
    bool gameOver;
};

#endif /* tictactoe_hpp */

tictactoe.cpp

#include "tictactoe.hpp"


class Tictactoe::Tictactoe {
    // Board for tictactoe
    int board[3][3] = {
        {0,0,0},
        {0,0,0},
        {0,0,0}
    };
    // Gandalf begins
    int turn = 1;
    // Get place position on board from user
    int pos[2] = {0,0};
    // for the gamePlay loop
    bool gameOver = false;
};

Tictactoe::Tictactoe() {};
Tictactoe::~Tictactoe() {};

// Function to place user token on given position
int Tictactoe::placeToken(int y, int x, int token){
    board[y][x] = token;
    return 0;
}

// Checking if the given position is already taken or not.
bool Tictactoe::positionIsTaken(int y, int x){
    if(board[y][x] == 0){
        return false;
    } else {
        return true;
    }
}

// Check if player won horizontally
bool Tictactoe::checkHorizontal(int token){
    for(int i = 0; i <= 2; i++){
        if( (board[i][0] == token) && (board[i][1] == token) && (board[i][2] == token) ){
            return true;
        }
    }
    return false;
}

// Check if player won vertically
bool Tictactoe::checkVertical(int token){
    for(int i = 0; i <= 2; i++){
        if ((board[0][i] == token) && (board[1][i] == token) && (board[2][i] == token)){
            return true;
        }
    }
    return false;
}


// Check if player won diagonally
bool Tictactoe::checkDiagonal(int token) {
    if((board[0][2] == token) && (board[1][1] == token) && (board[2][0] == token)){
        return true;
    }
    return false;
}

bool Tictactoe::playerWon(int token){
    return checkDiagonal(token) || checkVertical(token) || checkHorizontal(token);
}

bool Tictactoe::isTie(){
    for(int row = 0;row <= 2; row++){
        for(int col = 0;col <= 2; col++){
            if(board[row][col] == 0){
                return false;
            }
        }
    }
    return true;
}

// for printing board
void Tictactoe::showBoard(){
    cout << "\t-----------------\n";
    for(int row = 0;row <= 2; row++){
        for(int col = 0;col <= 2; col++){
            cout << "\t" << board[row][col] << "\t";
        }
        cout << endl;
    }
    cout << "\t-----------------\n";
};

void Tictactoe::gamePlay(){
    while(!gameOver){
        cout << ((turn == 1) ? "\t#############\n\tGandalf's Turn\n\t#############":"\t#############\n\tSaruman's Turn\n\t#############") << endl;
        if(turn == 1){

            // get User input
            cout << "Place Token (i.e: 0 2): ";
            cin >> pos[0] >> pos[1];

            // Ask for new input if position is already taken
            while(positionIsTaken(pos[0], pos[1])) {
                cout << "Position is taken, try again..." << endl;
                cout << "Place Token (i.e: 0 2): ";
                cin >> pos[0] >> pos[1];
            }

            // Set token on given position
            board[pos[0]][pos[1]] = 1;
            showBoard();
            // If player won horizontally
            if(isTie()){
                cout << "HA! Losers!" << endl;
                gameOver = true;
            }

            if(playerWon(1)){
                cout << "Gandalf Won." << endl;
                gameOver = true;
            }
            turn = 0;

        } else {
            // get User input
            cout << "Place Token (i.e: 0 2): ";
            cin >> pos[0] >> pos[1];

            // Ask for new input if position is already taken
            while(positionIsTaken(pos[0], pos[1])) {
                cout << "Position is taken, try again..." << endl;
                cout << "Place Token (i.e: 0 2): ";
                cin >> pos[0] >> pos[1];
            }

            // Set token on given position
            board[pos[0]][pos[1]] = 2;
            showBoard();
            if(isTie()){
                cout << "HA! Losers!" << endl;
                gameOver = true;
            }
            // If player won horizontally
            if(playerWon(2)){
                cout << "Saruman Won." << endl;
                gameOver = true;
            }
            turn = 1;
        }
    }
}

void Tictactoe::intro(){
    char isReady;
    cout << "\t##################################\n\t# Welcome to the TicTacToe Game! #\n\t##################################" <<
    "\nPlayer 1 (Gandalf)\nPlayer 2 (Saruman)\n\nAre you ready to begin?" << endl;
    cin >> isReady;
    if(isReady == 'Y' || isReady == 'y'){
        gamePlay();
    }
}
\$\endgroup\$
1
\$\begingroup\$

Naming:

  • I would rename the intro function to 'run' or 'play' as it runs or plays the game, not just the intro.
  • showBoard -> printBoard and delete the comment

Misc.

1) Instead of

bool Tictactoe::positionIsTaken(int y, int x){
    if(board[y][x] == 0){
        return false;
    } else {
        return true;
    }
}

You could just return the result directly:

bool Tictactoe::positionIsTaken(int y, int x){
    return board[y][x] != 0;
}

2) To ensure that a user does not for example input -5000 or 1000 in placeToken i would change the board data-type to an enum to store the state of each square on the board:

enum SquareStatus { Empty, Cross, Circle };

3) placeReturn should properly return void or an bool if the move was succesful instead of always just returning 0.

4) I would add a constructor argument containing the size of the game-board, making it possible to use a different sized board. This would also require the win calculation to change to be dynamic.

5) It looks like you're only checking one diagonal

6) There's a lot of duplicated code in the gamePlay

\$\endgroup\$

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.