5
\$\begingroup\$

I feel that I have done as much as my knowledge base allows me to on developing this tic tac toe program. I would love for anyone to critique it and tell me of any good/bad points.

Includes two players, optional AI, optional first turn and a scoring system.

The two things that I am unsure about are the 'GameLogic' class has a lot of functions but they do seem some what relevant. The second is the amount of classes I am sending as parameters, is this bad practice and is there another way to do it?

'Main.cpp'

// Tic Tac Toe.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "PlayingBoard.h"
#include "GameLogic.h"
#include "computerPlayer.h"
#include "humanPlayer.h"



int main()
{
    computerPlayer computer;
    humanPlayer player1, player2;
    GameLogic gameLogic;
    PlayingBoard playingBoard;

    //The game
    gameLogic.game(player1, player2, computer, gameLogic, playingBoard);


    std::string barn;
    std::cin >> barn;

    return 0;
}

'PlayingBoard.h'

#pragma once

#include <iostream>
#include <string>

class PlayingBoard
{
private:
    char m_playingBoard[]; // The playing board

public:
    PlayingBoard();
    static const int NUMBER_OF_SQUARES = 9;
    static const char EMPTY = ' '; //Empty game piece
    static const char X = 'X'; //Game piece: 'X'
    static const char O = 'O'; //Game piece: 'O'

    void initialize();
    const void display();
    const char getBoard(int i);
    void addGamePiece(int movePosition, char gamePiece);
    void removeGamePiece(int movePosition);
    void clear();

};

'PlayingBoard.cpp'

#include "stdafx.h"
#include "PlayingBoard.h"


PlayingBoard::PlayingBoard()
{
    m_playingBoard[NUMBER_OF_SQUARES];
    initialize();
}


void PlayingBoard::initialize()
{

    int addToBoard = 0;
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        m_playingBoard[addToBoard] = EMPTY;
        addToBoard++;
    }
}

const void PlayingBoard::display()
{
    int numb = 0;
    const int newRow = 3;
    std::cout << "---------------------" << "\n";
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        std::cout << " | " << m_playingBoard[i] << " | ";
        numb++;

        //Makes new row in board
        if (numb % newRow == 0)
        {
            std::cout << "\n";
            std::cout << "---------------------";
            std::cout << "\n";
        }
    }
}

const char PlayingBoard::getBoard(int i)
{
    return m_playingBoard[i];
}

void PlayingBoard::addGamePiece(int movePosition, char gamePiece)
{
    m_playingBoard[movePosition] = gamePiece;
}

void PlayingBoard::removeGamePiece(int movePosition)
{
    m_playingBoard[movePosition] = EMPTY;
}


void PlayingBoard::clear()
{
    int elementNumb = 0; //Element number that it is going through
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        m_playingBoard[elementNumb] = EMPTY;
        elementNumb++;
    }
}

'GameLogic.h'

#pragma once

#include <iostream>

class PlayingBoard;
class computerPlayer;
class humanPlayer;

class GameLogic
{
private:
    const int m_GameWinningScore = 3;
    const int m_winRoundPoint = 1;
    int m_NumbOfRounds = 1;

public:
    static const char TIE = 'T'; //Game is tie
    static const char NOONE = 'N'; //No one has won the game yet

    bool isMoveLegal(PlayingBoard& playingBoard, int move);
    bool isGameOver(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer);

    bool decideFirstTurn(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer);
    bool isComputerInPlay();
    bool isGameTie(PlayingBoard& playingBoard);
    bool isRoundOver(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, PlayingBoard& playingBoard);


    char findWinner(PlayingBoard& playingBoard);
    char decideTurn(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, char gamePiece);
    char initializeFirstTurn();


    void allocateGamePieces(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, bool firstTurn);
    void game(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard);
    void gameRound(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard);

    void announceRoundWinner(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard, char winner);
    void announceGameWinner(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer);
    void scoreManagement(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, char winner);
    void printScores(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer);
    void printRound();
    void incrementRoundNumb();
};

'GameLogic.cpp'

#include "stdafx.h"

#include "GameLogic.h"
#include "PlayingBoard.h"
#include "computerPlayer.h"
#include "humanPlayer.h"



bool GameLogic::decideFirstTurn(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer)
{
    bool firstTurn;
    char responce = ' ';

    do
    {
        std::cout << "Would you like to go first? 'y' - Yes. 'n' - No." << "\n";
        std::cin >> responce;
        switch (responce)
        {
        case 'y':
            firstTurn = true;
            break;

        case 'n':
            firstTurn = false;
            break;

        default:
            std::cout << "Please enter valid data." << "\n";
        }
    } while (responce != 'y' && responce != 'n');

    allocateGamePieces(player1, player2, computer, firstTurn);

    if (firstTurn)
        return true;
    else
        return false;

}

void GameLogic::allocateGamePieces(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, bool firstTurn)
{
    if (computer.isInPlay())
    {
        if (firstTurn)
        {
            player1.setGamePiece(PlayingBoard::X);
            computer.setGamePiece(PlayingBoard::O);
        }

        else
        {
            computer.setGamePiece(PlayingBoard::X);
            player1.setGamePiece(PlayingBoard::O);
        }
    }

    else
    {
        if (firstTurn)
        {
            player1.setGamePiece(PlayingBoard::X);
            player2.setGamePiece(PlayingBoard::O);
        }

        else
        {
            player2.setGamePiece(PlayingBoard::X);
            player1.setGamePiece(PlayingBoard::O);
        }
    }
}

bool GameLogic::isComputerInPlay()
{
    char responce = ' ';
    std::cout << "Would you like to face a human or computer. 'h' Human. 'c' Computer.";
    std::cin >> responce;

    switch (responce)
    {
    case 'h':
        return false;
        break;

    case 'c':
        return true;
        break;

    default:
        std::cout << "Please enter valid data." << "\n";
    }
}

char GameLogic::findWinner(PlayingBoard& playingBoard)
{
    //All combination of winnngRows
    const int winningRows[8][3] = {
        //Horizontal 
        { 0, 1, 2 },
        { 3, 4, 5 },
        { 6, 7, 8 },
        //Vertical
        { 0, 3, 6 },
        { 1, 4, 7 },
        { 2, 5, 8 },
        //Diagonal
        { 0, 4, 8 },
        { 2, 4, 6 }
    };


    const int totalRows = 8; //Amount of rows there are to search through

    //playingBoard.getBoard(winningRows[row][0]);
    for (int row = 0; row < totalRows; row++)
    {
        if ((playingBoard.getBoard(winningRows[row][0]) != PlayingBoard::EMPTY) &&
            (playingBoard.getBoard(winningRows[row][0]) == playingBoard.getBoard(winningRows[row][1])) &&
            (playingBoard.getBoard(winningRows[row][1]) == playingBoard.getBoard(winningRows[row][2])))
        {
            return playingBoard.getBoard(winningRows[row][0]);
        }

    }

    if (isGameTie(playingBoard))
    {
        return TIE;
    }

    return NOONE;
}

bool GameLogic::isGameTie(PlayingBoard& playingBoard)
{
    int i = 0;
    for (int i = 0; i < PlayingBoard::NUMBER_OF_SQUARES; i++)
    {
        if (playingBoard.getBoard(i) == PlayingBoard::EMPTY)
        {
            return false;
        }
    }

    return true;
}

bool GameLogic::isRoundOver(humanPlayer & player1, humanPlayer & player2, computerPlayer& computer, PlayingBoard& playingBoard)
{
    return
        findWinner(playingBoard) == player1.getGamePiece() ||
        findWinner(playingBoard) == player2.getGamePiece() ||
        findWinner(playingBoard) == computer.getGamePiece();
}

bool GameLogic::isMoveLegal(PlayingBoard& playingBoard, int move)
{
    return playingBoard.getBoard(move) == PlayingBoard::EMPTY;
}

bool GameLogic::isGameOver(humanPlayer & player1, humanPlayer & player2, computerPlayer & computer)
{
    return
        player1.getScore() >= m_GameWinningScore ||
        player2.getScore() >= m_GameWinningScore ||
        computer.getScore() >= m_GameWinningScore;
}

char GameLogic::decideTurn(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, char gamePiece)
{
    if (computer.isInPlay())
    {
        if (gamePiece == player1.getGamePiece())
            return computer.getGamePiece();

        else
            return player1.getGamePiece();
    }

    else
    {
        if (gamePiece == player1.getGamePiece())
            return player2.getGamePiece();

        else
            return player1.getGamePiece();
    }
}

char GameLogic::initializeFirstTurn()
{
    return PlayingBoard::X;
}

void GameLogic::game(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard)
{
    computer.setInPlay(gameLogic.isComputerInPlay());

    while (!isGameOver(player1, player2, computer))
    {
        printRound();
        gameRound(player1, player2, computer, gameLogic, playingBoard);
    }

    announceGameWinner(player1, player2, computer);

    //announce game winner
}

void GameLogic::gameRound(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard)
{
    char turn = initializeFirstTurn();
    gameLogic.decideFirstTurn(player1, player2, computer);
    playingBoard.display();

    while (!isRoundOver(player1, player2, computer, playingBoard))
    {
        if (turn == player1.getGamePiece())
        {
            player1.move(playingBoard, gameLogic);
            turn = decideTurn(player1, player2, computer, player1.getGamePiece());
        }

        else if (turn == player2.getGamePiece())
        {
            player2.move(playingBoard, gameLogic);
            turn = decideTurn(player1, player2, computer, player2.getGamePiece());
        }

        else if (turn == computer.getGamePiece())
        {
            computer.move(playingBoard, gameLogic, player1);
            turn = decideTurn(player1, player2, computer, computer.getGamePiece());
        }
        //Display board
        playingBoard.display();
    }

    //After every round ends
    char winner = findWinner(playingBoard);
    announceRoundWinner(player1, player2, computer, gameLogic, playingBoard, winner);
    scoreManagement(player1, player2, computer, winner);
    playingBoard.clear();

    printScores(player1, player2, computer);

    incrementRoundNumb();

}

void GameLogic::announceRoundWinner(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer, GameLogic& gameLogic, PlayingBoard& playingBoard, char winner)
{
    if (winner == player1.getGamePiece())
    {
        std::cout << "Player 1 has wins the round." << "\n";
    }

    else if (winner == player2.getGamePiece())
    {
        std::cout << "Player 2 wins the round." << "\n";
    }

    else if (winner == computer.getGamePiece())
    {
        std::cout << "The Computer wins the round." << "\n";
    }
}

void GameLogic::scoreManagement(humanPlayer & player1, humanPlayer & player2, computerPlayer& computer, char winner)
{
    if (winner == player1.getGamePiece())
        player1.addScore(m_winRoundPoint);

    else if (winner == player2.getGamePiece())
        player2.addScore(m_winRoundPoint);

    else if (winner == computer.getGamePiece())
        computer.addScore(m_winRoundPoint);

}

void GameLogic::printScores(humanPlayer & player1, humanPlayer & player2, computerPlayer& computer)
{
    if (computer.isInPlay())
    {
        std::cout << "\nPlayer1 Score: " << player1.getScore() << "\n";
        std::cout << "Computer Score:" << computer.getScore() << "\n";
    }

    else
    {
        std::cout << "\nPlayer1 Score: " << player1.getScore() << "\n";
        std::cout << "Player2 Score: " << player2.getScore() << "\n\n";
    }

}

void GameLogic::incrementRoundNumb()
{
    m_NumbOfRounds++;
}

void GameLogic::announceGameWinner(humanPlayer& player1, humanPlayer& player2, computerPlayer& computer)
{
    if (computer.isInPlay())
    {
        if (player1.getScore() >= m_GameWinningScore)
            std::cout << "Player1 wins the game in " << m_NumbOfRounds << " rounds!" << "\n";

        if (computer.getScore() >= m_GameWinningScore)
            std::cout << "Computer wins the game in " << m_NumbOfRounds << " rounds!" << "\n";
    }

    else
    {
        if (player1.getScore() >= m_GameWinningScore)
            std::cout << "Player1 wins the game in " << m_NumbOfRounds << " rounds!" << "\n";

        if (player2.getScore() >= m_GameWinningScore)
            std::cout << "Player2 wins the game in " << m_NumbOfRounds << " rounds!" << "\n";
    }

}

void GameLogic::printRound()
{
    std::cout << "\nRound " << m_NumbOfRounds << "\n";
}

'Player.h'

#pragma once
#include <iostream>
class Player
{
private:
    char m_GamePiece = ' ';
    int m_Score = 0;
    bool m_firstTurn;

public:

    const void setGamePiece(char piece);
    const char getGamePiece();
    void addScore(int score);
    int getScore();
};

'Player.cpp'

#include "stdafx.h"
#include "Player.h"


const void Player::setGamePiece(char piece)
{
    m_GamePiece = piece;
}

const char Player::getGamePiece()
{
    return m_GamePiece;
}

void Player::addScore(int score)
{
    m_Score += score;
}

int Player::getScore()
{
    return m_Score;
}

'computerPlayer.h'

#pragma once
#include "Player.h"
#include <vector>


class PlayingBoard;
class GameLogic;
class humanPlayer;

class computerPlayer :
    public Player
{
private:
    bool m_InPlay;

public:

    void setInPlay(bool play);
    bool isInPlay();
    void move(PlayingBoard& playingBoard, GameLogic& gameLogic, humanPlayer& player1);
    int getMovePosition(PlayingBoard& playingBoard, GameLogic& gameLogic, humanPlayer& player1);
    void winNextTurn(PlayingBoard& playingBoard, GameLogic& gameLogic, int& move, bool& found);
    void blockNextTurn(PlayingBoard& playingBoard, GameLogic& gameLogic, humanPlayer& player1, int& move, bool& found);
    void bestNextPosition(PlayingBoard& playingBoard, GameLogic& gameLogic, int& move, bool& found);

};

'computerPlayer.cpp'

#include "stdafx.h"
#include "computerPlayer.h"
#include "PlayingBoard.h"
#include "GameLogic.h"
#include "humanPlayer.h"

void computerPlayer::setInPlay(bool play)
{
    m_InPlay = play;
}

bool computerPlayer::isInPlay()
{
    return m_InPlay;
}

int computerPlayer::getMovePosition(PlayingBoard & playingBoard, GameLogic& gameLogic, humanPlayer& player1)
{
    int movePosition = 0;
    bool found = false; //Found position to move

    while (!found)
    {
        winNextTurn(playingBoard, gameLogic, movePosition, found);
        blockNextTurn(playingBoard, gameLogic, player1, movePosition, found);
        bestNextPosition(playingBoard, gameLogic, movePosition, found);
    }

    //movePosition = 4;
    return movePosition;
}

void computerPlayer::winNextTurn(PlayingBoard& playingBoard, GameLogic& gameLogic, int& move, bool& found)
{
    if (!found)
    {
        move = 0; // Reset movement
        while (move < PlayingBoard::NUMBER_OF_SQUARES && !found)
        {
            if (gameLogic.isMoveLegal(playingBoard, playingBoard.getBoard(move)))
            {
                playingBoard.addGamePiece(move, getGamePiece());
                found = gameLogic.findWinner(playingBoard) == getGamePiece();
                playingBoard.removeGamePiece(move);
            }

            else
            {
                move++;
            }
        }
    }

    else
    {
        return;
    }
}

void computerPlayer::blockNextTurn(PlayingBoard& playingBoard, GameLogic& gameLogic, humanPlayer& player1, int& move, bool& found)
{
    if (!found)
    {
        move = 0; // Reset movement position
        while (move < PlayingBoard::NUMBER_OF_SQUARES && !found)
        {
            if (gameLogic.isMoveLegal(playingBoard, playingBoard.getBoard(move)))
            {
                playingBoard.addGamePiece(move, player1.getGamePiece());
                found = gameLogic.findWinner(playingBoard) == player1.getGamePiece();
                playingBoard.removeGamePiece(move);
            }

            else
            {
                move++;
            }
        }
    }

    else
    {
        return;
    }
}

void computerPlayer::bestNextPosition(PlayingBoard& playingBoard, GameLogic& gameLogic, int& move, bool& found)
{
    if (!found)
    {
        /*
        0, 1, 2
        3, 4, 5
        6, 7, 8
        */
        int i = 0;
        move = 0;
        std::vector<int> bestMoves = { 4, 0, 2, 6, 8, 1, 3, 5, 7 };

        while (!found && i < bestMoves.size())
        {
            move = bestMoves[i];
            if (gameLogic.isMoveLegal(playingBoard, bestMoves[i]))
            {
                found = true;
            }

            else
            {
                i++;
            }
        }

    }

    else
    {
        return;
    }
}

void computerPlayer::move(PlayingBoard& playingBoard, GameLogic& gameLogic, humanPlayer& player1)
{
    int movePosition = getMovePosition(playingBoard, gameLogic, player1);

    playingBoard.addGamePiece(movePosition, getGamePiece());
}

'humanPlayer.h'

#pragma once
#include "Player.h"
class PlayingBoard;
class GameLogic;

class humanPlayer :
    public Player
{
public:

    int getMovePosition(PlayingBoard& playingBoard, GameLogic& gameLogic);
    void move(PlayingBoard& playingBoard, GameLogic& gameLogic);
};

'humanPlayer.cpp'

#include "stdafx.h"
#include "humanPlayer.h"
#include "PlayingBoard.h"
#include "GameLogic.h"


int humanPlayer::getMovePosition(PlayingBoard& playingBoard, GameLogic& gameLogic)
{
    int low = 0,
        high = PlayingBoard::NUMBER_OF_SQUARES - 1,
        movePosition = 0;
    bool moveAllowed = false;
    do
    {
        std::cout << "Enter a number between " << low << " and " << high << ".\n";
        std::cin >> movePosition;

        if (gameLogic.isMoveLegal(playingBoard, movePosition))
            moveAllowed = true;
        else
            std::cout << "Square already occupied." << "\n";

    } while (!moveAllowed);

    return movePosition;
}

void humanPlayer::move(PlayingBoard& playingBoard, GameLogic& gameLogic)
{
    int move = getMovePosition(playingBoard, gameLogic);

    playingBoard.addGamePiece(move, getGamePiece());
}
\$\endgroup\$
3
\$\begingroup\$

Design

IMHO, I don't think making the game logic a class is the right way to go here. Generally, when I think of a class, I think of an object that maintains some kind of internal state that the client is able to carefully interact with using a public interface. Your GameLogic class is essentially a collection of related functions which is perfect for a header/implementation file duo, not a class. I would change your GameLogic class to be something like:

Game.hpp

namespace game {

void initializeFirstTurn();
....
....
....
}

Game.cpp

#include "Game.hpp"

game::initializeFirstTurn()
{
    // ...
}
...
...

Moreover, you shouldn't construct a player/computer object until you know whether two human players or one human/one AI player are playing against each other. So, in the main function, I would ask to see what the user wanted to do and only then do I construct the appropriate objects. As it is now, you're constructing two player objects and a computer object that might not be playing.

int main()
{
    cout << "1 or 2 player?\n";
    bool onePlayer = GetOneOrTwoPlayer();

    if (onePlayer)
        InitializeOnePlayerGame(); // construct players here
    else
        InitializeTwoPlayerGame(); // construct players here
}

I also think it would make it easier if you made the private fields of the Player class protected so that the subclasses can directly access them. This way, we can have a constructor that allows us to isntantly initialize each Player to a game piece:

humanPlayer::humanPlayer(char piece) : m_GamePiece(piece) {}

humanPlayer p1('X');

Moreover, many of the functions that computerPlayer has look more like GameLogic functionality to me. I would try to factor that out and make the Player classes only house code that is strictly necessary for the player.

Code Redundancy

void PlayingBoard::clear()
{
    int elementNumb = 0; //Element number that it is going through
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        m_playingBoard[elementNumb] = EMPTY;
        elementNumb++;
    }
}

void PlayingBoard::initialize()
{

    int addToBoard = 0;
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        m_playingBoard[addToBoard] = EMPTY;
        addToBoard++;
    }
}

Notice something about this code? It's exactly the same. We can remove the need for the initialize() function. Also, we can put addtoBoard / elemNumb inside the for loop and increment there.

EDIT: Actually, there's no need for the additional addtoBoard / elemNumb variable. Just use i.

void PlayingBoard::clear()
{
    for (int i = 0; i < NUMBER_OF_SQUARES; i++)
    {
        m_playingBoard[i] = EMPTY;
    }
}

PlayingBoard::PlayingBoard()
{
    m_playingBoard[NUMBER_OF_SQUARES];
    clear();
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for this! Regarding namespaces, do you know of any good places I can read up on them? I also read that using protected functions was considered to be bad practice. I am not sure why this is because I was going to use them in this program but hearing that comment put me off. Is that just totally nonsense and completely project dependent? \$\endgroup\$ – Ryan Swann Jan 12 '16 at 8:01
  • 1
    \$\begingroup\$ en.cppreference.com/w/cpp/language/namespace is a pretty decent link to read up on namespaces. The reason protected members are sometimes considered bad is that protected members are considered public within the scope of all inheriting classes. For example, if you have a protected member int numVals and numVals is supposed to be >= 5 && <= 2, there is no guarantee that someone inheriting from your class will set it to an appropriate value. \$\endgroup\$ – Bizkit Jan 12 '16 at 15:02
  • \$\begingroup\$ That's great, thank you. May I ask; how far would this go in adding this to a github and having it as an example of what I can do to an employer? I am currently teaching my self C++ and wondering how far a portfolio can go. Thank you for all your help. \$\endgroup\$ – Ryan Swann Jan 12 '16 at 20:05
  • 1
    \$\begingroup\$ No problem. As far as adding this to github for employer review, I would at least think over the class design as I mentioned in my review. For example, remove the GameLogic class and implement it as free-standing functions. I would also rethink the Player class hierarchy. Think very carefully about what the real difference between an AI and human player is.... it's how they input the next move. Think of how to build a heirarchy out of that. Message me if you want to discuss further as ~500 characters isn't enough to cover this :) \$\endgroup\$ – Bizkit Jan 12 '16 at 22:01

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.