# Tic Tac Toe OOP, need advice on whole game design

## Recommended Posts

Hi everyone. I finished Tic Tac Toe for the second time, this time using classes. I would like your opinions on the game design, things I'm not doing well, etc. The game constists of 3 base classes and 2 derived ones. Those are: TicTacToe: the game Board: represents a board GenericPlayer: a generic player (abstract class) HumanPlayer and ComputerPlayer: these are derived from generic player. <br/> TicTacToe.h
#ifndef TIC_TAC_TOE_H
#define TIC_TAC_TOE_H

#include <cassert> // used for debugging
#include <iostream>
#include <string>
#include <ctime> // used to a random number to decide which player chooses piece first

//constants needed
#include "constants.h"

//forward class declarations
class Board;
class GenericPlayer;
class HumanPlayer;
class ComputerPlayer;

class TicTacToe
{
public:
TicTacToe(int numberOfPlayers = 1);
~TicTacToe();

void Play(); // play a game of TicTacToe
void Instructions();

bool Tie() const; // true if it's a tie
char Win() const; // returns the winning piece if there's a winner, or empty if there isn't a winner

private:
void SetPieces(); // sets the pieces, e.g. X for p1

Board *board;
GenericPlayer *p1, *p2;
};

#endif //TIC_TAC_TOE_H


TicTacToe.cpp
#include "TicTacToe.h"

#include "Board.h"
#include "Player.h"

TicTacToe::TicTacToe(int numberOfPlayers)
{
board = new Board();

assert(numberOfPlayers == 1 || numberOfPlayers == 2);

if(numberOfPlayers == 1)
{
p1 = new HumanPlayer();
p2 = new ComputerPlayer();
dynamic_cast <ComputerPlayer*> (p2)->SetBoard(board);
}
else
{
p1 = new HumanPlayer();
p2 = new HumanPlayer();
}

std::cout << "\n";
}

TicTacToe::~TicTacToe()
{
//free the memory allocated by the GenericPlayer pointers
delete p1;
delete p2;

//and the board
delete board;
}

void TicTacToe::Instructions()
{
std::cout << "Welcome to Tic Tac Toe!!\n\n";

board->SetField(0, '1');
board->SetField(1, '2');
board->SetField(2, '3');

board->SetField(3, '4');
board->SetField(4, '5');
board->SetField(5, '6');

board->SetField(6, '7');
board->SetField(7, '8');
board->SetField(8, '9');

std::cout << "You enter a value from 1 to 9 to indicate where you want to play, as illustratedbelow:\n\n";
std::cout << *board;

board->Clear();
}

void TicTacToe::Play()
{
SetPieces();

GenericPlayer *whoPlays = 0;

if(p1->GetPiece() == X)
whoPlays = p1;
else
whoPlays = p2;

//THE GAME LOOP
int choice;
while(Win() == EMPTY && !Tie())
{
// if it's a human player that's playing, display the text
if(dynamic_cast <HumanPlayer*> (whoPlays))
std::cout << whoPlays->GetName() << ", it's your turn. [" << whoPlays->GetPiece() << "] 1-9: ";

// loop until the player who plays chooses a field which is empty
do
{
choice = whoPlays->makeMove();
if(board->GetField(choice) != EMPTY)
std::cout << "Hey, you can't place it there..!\n";

} while(board->GetField(choice) != EMPTY);

std::cout << whoPlays->GetName() << " chooses " << choice + 1 << ".\n\n";

board->SetField(choice, whoPlays->GetPiece());
std::cout << *board;

// it's the turn of the other player
if(whoPlays == p1)
whoPlays = p2;
else
whoPlays = p1;
}

if(Win() == p1->GetPiece())
std::cout << p1->GetName() << " wins! Sorry " << p2->GetName() << ", better luck next time...\n";
if(Win() == p2->GetPiece())
std::cout << p2->GetName() << " wins! Sorry " << p1->GetName() << ", better luck next time...\n";
}

//returns true if it's a tie
bool TicTacToe::Tie() const
{
//if the fields that do not contain EMPTY are ROWS * COLUMNS and no one has won, it's a tie
if(board->NumFieldsNotChar(EMPTY) == ROWS * COLUMNS)
{
std::cout << "It's a tie!!\n";
return true;
}

return false;
}

char TicTacToe::Win() const
{
for(int i = Board::HORIZONTAL; i <= Board::DIAGONAL; ++i)
{
// i here needs to be casted to Board::DIRECTION
if(board->Consecutive3((Board::DIRECTION)i, p1->GetPiece()))
return p1->GetPiece();
if(board->Consecutive3((Board::DIRECTION)i, p2->GetPiece()))
return p2->GetPiece();
}

return EMPTY;
}

void TicTacToe::SetPieces()
{
srand((unsigned)time(0));
int p1First = rand() % 2; // if 1, p1 plays first. if 0, p2 plays first (gets X, which goes first)

if(p1First)
{
std::cout << p1->GetName() << " plays first. [X]\n\n";
p1->SetPiece(X);
p2->SetPiece(O);
}
else
{
std::cout << p2->GetName() << " plays first. [X]\n\n";
p1->SetPiece(O);
p2->SetPiece(X);
}
}


This is the board class. Board.h
#ifndef BOARD_H
#define BOARD_H

#include <iostream>
#include <cassert>

#include "constants.h"
#include "functions.h"

class Board
{
public:
Board();

void Clear(); // clear the table (set all fields to EMPTY, defined in constants.h

friend std::ostream &operator <<(std::ostream &os, const Board &b);

//set a field to c
void SetField(int row, int column, char c);
void SetField(int index, char c);

char GetField(int row, int column) const;
char GetField(int index) const;

//returns the number of fields that do not contain c
int NumFieldsNotChar(char c);

enum DIRECTION {HORIZONTAL, VERTICAL, DIAGONAL};
// returns true if c is found 3 times horizontal, vertical or diagonal
//e.g. Consecutive3(Board::HORIZONTAL, 'X')
// O X O
// X O X
// X X X
// it will return true
bool Consecutive3(DIRECTION direction, char c);

private:
char table[COLUMNS][ROWS];
};

#endif //BOARD_H


Board.cpp
#include "Board.h"

Board::Board()
{
Clear();
}

void Board::Clear()
{
for(int i = 0; i < ROWS; ++i)
for(int j = 0; j < COLUMNS; ++j)
table[i][j] = EMPTY;
}

std::ostream &operator << (std::ostream &os, const Board &b)
{
for(int i = 0; i < ROWS; ++i)
{
for(int j = 0; j < COLUMNS; ++j)
{
std::cout << b.table[i][j];

if(j != ROWS - 1)
std::cout << " | ";
}

std::cout << "\n";

if(i != ROWS - 1)
{
for(int k = 0; k < ROWS; ++k)
{
if(k != ROWS - 1)
std::cout << "-----";
}
}

std::cout << "\n";
}

return os;
}

void Board::SetField(int row, int column, char c)
{
boundCheck(0, ROWS, row);
boundCheck(0, ROWS, column);
table[row][column] = c;
}

void Board::SetField(int index, char c)
{
boundCheck(0, ROWS * ROWS, index);
int i, j;
convert1Dto2D(index, i, j);
table[i][j] = c;
}

char Board::GetField(int row, int column) const
{
boundCheck(0, ROWS, row);
boundCheck(0, ROWS, column);
return table[row][column];
}

char Board::GetField(int index) const
{
boundCheck(0, ROWS * ROWS, index);
int i, j;
convert1Dto2D(index, i, j);
return table[i][j];
}

int Board::NumFieldsNotChar(char c)
{
int count = 0;

for(int i = 0; i < ROWS; ++i)
for(int j = 0; j < COLUMNS; ++j)
if(table[i][j] != c)
++count;

return count;
}

bool Board::Consecutive3(Board::DIRECTION direction, char c)
{
switch(direction)
{
case Board::HORIZONTAL:

for(int i = 0; i < ROWS; ++i)
{
if(GetField(i, 0) == GetField(i, 1) && GetField(i, 0) == GetField(i, 2) )
if(GetField(i, 0) == c)
return true;
}

case Board::VERTICAL:
for(int i = 0; i < COLUMNS; ++i)
{
if(GetField(0, i) == GetField(1, i) && GetField(0, i) == GetField(2, i) )
if(GetField(0, i) == c)
return true;
}

case Board::DIAGONAL:
if(GetField(0, 0) == GetField(1, 1) && GetField(0, 0) == GetField(2, 2) )
if(GetField(0, 0) == c)
return true;

if(GetField(0, 2) == GetField(1, 1) && GetField(0, 2) == GetField(2, 0) )
if(GetField(0, 2) == c)
return true;
}

// horizontal

return false;
}


The player classes: Player.h
#ifndef PLAYER_H
#define PLAYER_H

#include <iostream> // used to get HumanPlayer name (getline() )
#include <string> // used to get HumanPlayer name
#include <ctime> // used for random input from ComputerPlayer

#include "constants.h"
#include "functions.h"

class Board; // used by ComputerPlayer

class GenericPlayer // abstract class
{
public:
virtual int makeMove() = 0;

const std::string & GetName() const;

void SetPiece(char c);
char GetPiece() const;

protected:
char piece; // the piece a player has, X or O
std::string name;
};
//////////////////////////////////////////////////////////////////////////////////////////////////////////
class HumanPlayer : public GenericPlayer
{
public:
//constructors
HumanPlayer(const std::string &_name);
HumanPlayer();

virtual int makeMove();
};
////////////////////////////////////////////////////////////////////////////////////////////////////
class ComputerPlayer : public GenericPlayer
{
public:
ComputerPlayer();
virtual int makeMove();

void SetBoard(const Board *const board);

private:
const Board *pBoard;
};

#endif //PLAYER_H


Player.cpp
#include "TicTacToe.h"

#include "Board.h"
#include "Player.h"

/******************************** GenericPlayer ***********************************************************/
void GenericPlayer::SetPiece(char c)
{
piece = c;
}

char GenericPlayer::GetPiece() const
{
return piece;
}

const std::string & GenericPlayer::GetName() const
{
return name;
}
/************************************* HumanPlayer ********************************************************/

HumanPlayer::HumanPlayer(const std::string &_name)
{
name = _name;
}

HumanPlayer::HumanPlayer()
{
std::cout << "Enter name: ";
std::getline(std::cin, name);
}

int HumanPlayer::makeMove()
{
int temp;

do
{
temp = getInt();
if(temp <= 0 || temp > ROWS * COLUMNS)
std::cout << temp << " is not between 1 and 9. Enter again: ";

}while(temp <=0 || temp > ROWS * COLUMNS);

return --temp;
}

/**************************************** ComputerPlayer **************************************************/

ComputerPlayer::ComputerPlayer(): pBoard(0)
{
name = "Computer";
}

int ComputerPlayer::makeMove()
{
srand((unsigned)time(0));

Board temp = *pBoard;

char myPiece = GetPiece();
char otherPiece;
if(myPiece == X)
otherPiece = O;
else
otherPiece = X;

// check for win
for(int i = 0; i < ROWS * COLUMNS; ++i)
{
if(temp.GetField(i) == EMPTY)
temp.SetField(i, myPiece);
else
continue;

//check if this position will make the computer win
for(int j = Board::HORIZONTAL; j <= Board::DIAGONAL; ++j)
if(temp.Consecutive3((Board::DIRECTION)j, myPiece))
return i;
//if not, undo the move
temp.SetField(i, EMPTY);
}

// check to stop the player from winning
for(int i = 0; i < ROWS * COLUMNS; ++i)
{
if(temp.GetField(i) == EMPTY)
temp.SetField(i, otherPiece);
else
continue;

//check if this position will make the player win and block it
for(int j = Board::HORIZONTAL; j <= Board::DIAGONAL; ++j)
if(temp.Consecutive3((Board::DIRECTION)j, otherPiece))
return i;

temp.SetField(i, EMPTY);
}

//else return a random number
int r;
do
{
r = rand() % (ROWS * COLUMNS);

} while(temp.GetField(r) != EMPTY);

return r;
}

void ComputerPlayer::SetBoard(const Board *const board)
{
pBoard = board;
}


Helper functions: functions.h
#ifndef FUNCTIONS_H
#define FUNCTIONS_H

#include <iostream>
#include <string>
#include <sstream>
#include <cassert> // used by boundCheck

#include "constants.h"

//throws a runtime error if val isn't between boundStart and boundEnd, boundEnd NOT included
void boundCheck(int boundStart, int boundEnd, int val);
//converts an index e.g. 6 to two values used in a 2D array, 6 is (2,0)
void convert1Dto2D(int index, int &row, int &column);
//this converts (2,0) to 6
void convert2Dto1D(int row, int column, int &index);
//read a string and return an int
int getInt();

#endif


functions.cpp
#include "functions.h"

void boundCheck(int boundStart, int boundEnd, int val)
{
assert(val >= boundStart && val < boundEnd);
}

void convert1Dto2D(int index, int &row, int &column)
{
row = int(index / ROWS);
column = index - row * ROWS;
}

void convert2Dto1D(int row, int column, int &index)
{
index = (row * ROWS) + column;
}

int getInt()
{
std::string temp;
std::getline(std::cin, temp);
std::stringstream ss(temp);
int i;
ss >> i;
return i;
}


and constants.h (global constants)
#ifndef CONSTANTS_H
#define CONSTANTS_H

const int COLUMNS = 3;
const int ROWS = 3;

const char EMPTY = ' ';
const char X = 'X';
const char O = 'O';

#endif

main.cpp
#include <iostream>

#include "functions.h"
#include "TicTacToe.h"

using namespace std;

int getNumberOfPlayers();

int main()
{
cout << "\tTic Tac Toe\tby Minas Mina\n\n";

TicTacToe game(getNumberOfPlayers());
game.Instructions();
game.Play();

cin.get();
return 0;
}

int getNumberOfPlayers()
{
cout << "How many players? 1/2: ";
int numPlayers;
do
{
numPlayers = getInt();

if(numPlayers != 1 && numPlayers != 2)
cout << "1 or 2, not " << numPlayers << "!\n";

}while(numPlayers != 1 && numPlayers != 2);
return numPlayers;
}
[/souurce]

Sorry for the long post..


[Edited by - sheep19 on December 15, 2008 9:38:33 AM]

##### Share on other sites
Design:

I would implement virtual int GenericPlayer::makeMove(const Board& board) = 0; that ask the move to the user in case of HumanPlayer, and calculate and return the move in case of CpuPlayer, all with a common interface and without dynamic_casts.

Implementation:

You should avoid the dynamic downcasts. You can declare virtual void SetBoard(const Board *const board) { } in GenericPlayer class to avoid the dynamic cast. Or do:
ComputerPlayer* p = new ComputerPlayer(); p->SetBoard(board); p2 = p;

You don't need to forward-declare HumanPlayer and ComputerPlayer in TicTacToe.h.

You don't need to pass const when you pass pointers by value (void SetBoard(const Board *const board)).

Since you always want to have a board, you should have a member of type Board, not Board*. If you did it not to include Board.h in TicTacToe.h, you would be better to declare a std::auto_ptr<Board> instead of a raw pointer, so you have no troubles if you forget to implement the destructor of TicTacToe. The same is valid for p1 and p2.

If style is more important than performance, you could replace
const char EMPTY = ' '; const char X = 'X'; const char O = 'O';
with
enum Entry { EMPTY, X, O }; std::map<Entry, char> EntryToChar; EntryToChar[EMPTY] = ' '; EntryToChar[X] = 'X'; EntryToChar[O] = 'O';

and have a Entry table[COLUMNS][ROWS]; in the Board.

You can move all includes from functions.h to functions.cpp, similar for TicTacToe.

Have fun!

##### Share on other sites
Quote:
 enum Entry { EMPTY, X, O }; std::map EntryToChar; EntryToChar[EMPTY] = ' '; EntryToChar[X] = 'X'; EntryToChar[O] = 'O';

Alternatively:
std::ostream &operator<<(std::ostream &out, Entry entry){    static const char lookup[] = { ' ', 'X', 'O' };    return out << lookup[entry];}

##### Share on other sites
I lack the time to make a detailed analysis, so I'll just make the odd few points [smile].

Generally if you have abstract polymorphic base class then it ought to have a virtual destructor even if it's not strictly needed, unless the intention is that the class objects are stateless by requirement.

The responsibilities seem a little fuddled, in your implementation you've mixed: 'who/what a player is', 'how a player is viewed' and 'how a player thinks' together into one class. This gives rise to complexity within your code; the best example of this is that I could not default construct an instance of a HumanPlayer without prompting the user for a name, heavens forbid I try and do that in a graphical application that has no console. This makes your player classes brittle and in need of a refactoring.

Similar comments to other portions of your code. I notice your Board class is limited to only 3x3, I know you've specified constants for the sizes but if you change them then it breaks your Consecutive3 function.

You also have some strange functions, like NumFieldsNotChar, which you call like this: board->NumFieldsNotChar(EMPTY). Wouldn't it make more sense just to have a predicate that checks whether the board is empty or not?: bool Empty()

Another strange function is Consecutive3. I don't understand what use the direction parameter is if the use case is simply to check whether a board is a winning board? You could just have a win predicate instead.

Anyway, I must get back to work.

##### Share on other sites
Some ideas:

- The Board is not polymorphic, so there is no reason to instantiate it dynamically.
- The main TicTacToe class buys you little, and introduces failure points and odd behaviour (why would I specify a number of players = 1 when that's really the number of *human* players? Why do I need to specify this *before* I can show the game instructions?). Just use free functions, and pass the player objects around by reference between functions. You will probably find that you don't even need to allocate dynamically, because you'll have local variables of a specific type that are passed everywhere by reference :)
- The Board has the best access to the cell data, so it should be what's responsible for detecting wins, ties and legal moves. The get/set interface is awkward. The AI should probably make its own copy (copies, for trying out moves) of the board's internal state, instead of grabbing a reference to the main Board.
- The code for complaining about moves probably belongs in the HumanPlayer; Player attempts to .move() will receive a return code indicating if the move was successful. (AI attempts can assert() success; Player failures would then write the message to cout - or better yet, an ostream& specified at construction.)
- Similarly, you could send .won() and .lost() messages to the Player objects; telling a computer AI "better luck next time" seems a bit strange :)

##### Share on other sites

I don't have the time now to read all the posts, I will do when I return home on Friday. (I'm a soldier)

Quote:
 Original post by ZahlmanSome ideas:- The Board is not polymorphic, so there is no reason to instantiate it dynamically.

Do you mean that Board in TicTacToe should be an actual object instead of a pointer?
If yes, I must #include "Board.h" instead of forward declaring the class, right?

Quote:
 - The main TicTacToe class buys you little, and introduces failure points and odd behaviour (why would I specify a number of players = 1 when that's really the number of *human* players?

Well, that's for human players...I made it like that because the player chooses that, and when someone plays a single player game, it's *1* player, that computer isn't included. At least that's what the player needs to know. Am I correct?[/quote]

Quote:
 Why do I need to specify this *before* I can show the game instructions?).

Well yeah, right. But the Player constructors are those who do that (promt for a name) Should I have a differenmt function, private to TicTacToe that creates the Player Objects? Like void TicTacToe::InitPlayers(int numberOfHumanPlayers)

Quote:
 Just use free functions, and pass the player objects around by reference between functions. You will probably find that you don't even need to allocate dynamically, because you'll have local variables of a specific type that are passed everywhere by reference :)

What do you mean? Can you show me an example in code?

Quote:
 - The Board has the best access to the cell data, so it should be what's responsible for detecting wins, ties and legal moves. The get/set interface is awkward. The AI should probably make its own copy (copies, for trying out moves) of the board's internal state, instead of grabbing a reference to the main Board.

Again, can you explain a bit more? Why make multiple copies? I did this so it just uses a const pointer (so it can't change that data of the table but only make a copy) and every time the function is called I create a local Board (copy of the main board) which is used to calculate the move and destroyed afterwards/ Why is this bad?

About having the Board detecting wins etc..Isn't Board just a "board"? Shouldn't it know only what it needs to, only that data it contains and provide some functions to access them? Shouldn't the game (TictacToe) be responsible for the wins? (mostly a design decision I think, but I would like opinions here.)

Quote:
 - The code for complaining about moves probably belongs in the HumanPlayer; Player attempts to .move() will receive a return code indicating if the move was successful. (AI attempts can assert() success; Player failures would then write the message to cout - or better yet, an ostream& specified at construction.)

So should .Move() return an int value? (or bool)? Should I display messages somewhere else? (so it could be used in a graphical environment like someone above said?)
Quote:
 - Similarly, you could send .won() and .lost() messages to the Player objects;

Why?
Quote:
 telling a computer AI "better luck next time" seems a bit strange :)

:)

I will read carefully all replies later. Thanks again :)

##### Share on other sites
Quote:
Original post by sheep19
Quote:
 Original post by ZahlmanSome ideas:- The Board is not polymorphic, so there is no reason to instantiate it dynamically.

Do you mean that Board in TicTacToe should be an actual object instead of a pointer?
If yes, I must #include "Board.h" instead of forward declaring the class, right?

Yes; and not if there is no longer such a TicTacToe class :)

C++ is not like Java or C#. Writing C++ in object-oriented style doesn't mean putting *everything* in an object. And anyway, not everything "is an object" in Java. In Python, functions, the classes themselves, and modules are all objects (and it goes even further than that actually). But a large percentage of normal Python code is written outside of any class.

Quote:
 Well, that's for human players...I made it like that because the player chooses that, and when someone plays a single player game, it's *1* player, that computer isn't included. At least that's what the player needs to know. Am I correct?

Decoding that is the responsibility of the code prompting for input.

Quote:
Quote:
 Why do I need to specify this *before* I can show the game instructions?).

Well yeah, right. But the Player constructors are those who do that (promt for a name) Should I have a differenmt function, private to TicTacToe that creates the Player Objects? Like void TicTacToe::InitPlayers(int numberOfHumanPlayers)

Ugh, now you want to make the class more complicated to use, while I want to just get rid of it. It's not a useful abstraction here. Live and learn. :)

Quote:
Quote:
 Just use free functions, and pass the player objects around by reference between functions. You will probably find that you don't even need to allocate dynamically, because you'll have local variables of a specific type that are passed everywhere by reference :)

What do you mean? Can you show me an example in code?

Later tonight.

Quote:
Quote:
 - The Board has the best access to the cell data, so it should be what's responsible for detecting wins, ties and legal moves. The get/set interface is awkward. The AI should probably make its own copy (copies, for trying out moves) of the board's internal state, instead of grabbing a reference to the main Board.

Again, can you explain a bit more? Why make multiple copies?

It depends how complicated your AI is. If it's trying out multiple possibilities, and needs to back-track, then it will want to make multiple copies. Often this is done implicitly: for example, searching for a move often uses a recursive function that passes a board by value (so it gets copied each time you step "into" the recursion, and the copy is destroyed when you step "out").

Quote:
 I did this so it just uses a const pointer (so it can't change that data of the table but only make a copy) and every time the function is called I create a local Board (copy of the main board) which is used to calculate the move and destroyed afterwards/ Why is this bad?

The const-ness fixes things considerably :) The general objection is that the Board has an awkward interface right now, and the AI has full access to that interface.

Quote:
 About having the Board detecting wins etc..Isn't Board just a "board"? Shouldn't it know only what it needs to, only that data it contains and provide some functions to access them? Shouldn't the game (TictacToe) be responsible for the wins? (mostly a design decision I think, but I would like opinions here.)

You can do that, but then there is really nothing for the Board to encapsulate and you can use a struct. Sometimes it's useful to pull things out at this level of detail, but practically, "storing the data" and "interpreting the data" are not usually considered separate responsibilities. :)

Just wait; I'll show a simple approach.

Quote:
Quote:
 - The code for complaining about moves probably belongs in the HumanPlayer; Player attempts to .move() will receive a return code indicating if the move was successful. (AI attempts can assert() success; Player failures would then write the message to cout - or better yet, an ostream& specified at construction.)

So should .Move() return an int value? (or bool)? Should I display messages somewhere else? (so it could be used in a graphical environment like someone above said?)

An enumeration would be a good way to handle it. Message display becomes the responsibility of the Player classes, which interpret the return value.

Quote:
 - Similarly, you could send .won() and .lost() messages to the Player objects;

Why?[/quote]

Because of the fact that you might want to customize the message depending on whether a Human or Computer won. Or consider the networking case; if you match two NetworkedHumanPlayers later on, you need to send "you won" and "you lost" messages separately over the internet.

Of course, it would be kind of silly to play tic-tac-toe over the internet. But if you want to learn OO design, I suppose it's as good a way as any.

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628301
• Total Posts
2981906

• 9
• 11
• 11
• 10
• 10