Reputation: 19
Im writing a simple tictactoe game with a simple ai. Im getting an error when initializing my game object. My game object initializes two player objects that each have their player number properties set to "one" or "two" with a method called setPlayerNum. Although I never use the "this" key word I am getting an error saying that "this was a nullptr" as well as a write access violation.
I've tried derefrencing the player object as well as making the player number property a public variable (in case it was a scope error). Neither worked for me.
int main() {
game g;
g.printTitleCard();
std::system("pause");
return 0;
}
#include "board.h"
#include "player.h"
class game {
public:
game();
~game();
void printTitleCard();
void play();
int victory();
void victoryScreen(int winner);
private:
board * gameBoard;
player * player_one;
player * player_two;
int turns;
int maxTurns = 9;
};
game::game() {
gameBoard = new board();
player_one->setPlayerNum(1);
player_two->setPlayerNum(2);
}
class player {
public:
player();
~player();
void setPlayerNum(int);
int getPlayerNum();
void updateScore(int);
int retrieveWins();
int retrieveLoses();
int retrieveTies();
private:
int playerNum = 0;
int wins = 0;
int loses = 0;
int ties = 0;
};
void player::setPlayerNum(int num) {
playerNum = num;
}
Upvotes: 0
Views: 367
Reputation: 33952
game::game() {
gameBoard = new board();
player_one->setPlayerNum(1);
player_two->setPlayerNum(2);
}
does not instantiate any player
s before calling setPlayerNum
, this means that there are no valid instances of player
to invoke setPlayerNum
on. After this the program has wandered into undefined behaviour and all bets are off.
Solution:
The best way to solve this is to go back a few steps to
class game {
public:
game();
~game();
void printTitleCard();
void play();
int victory();
void victoryScreen(int winner);
private:
board * gameBoard;
player * player_one;
player * player_two;
int turns;
int maxTurns = 9;
};
And NOT use pointers for gameBoard
, player_one
and player_two
.
board * gameBoard;
player * player_one;
player * player_two;
becomes
board gameBoard;
player player_one;
player player_two;
and
game::game() {
gameBoard = new board();
player_one->setPlayerNum(1);
player_two->setPlayerNum(2);
}
becomes
game::game() {
player_one.setPlayerNum(1);
player_two.setPlayerNum(2);
}
Only use new
when you absolutely have to, and in a world of library containers and smart pointers that's almost never. More on that here: Why should C++ programmers minimize use of 'new'?
Unrelated:
If you change the player constructor to take the player number as a parameter, setPlayerNum
likely becomes unnecessary and game
's constructor can become
game::game(): player_one(1), player_two(2){
}
Is there any way to keep them as pointers?
Yes, but it's not a very good idea because you pick up a tonne of extra responsibilities. You nave to new
the players in the constructor before you use them. You have to delete
the the players and game
in the destructor, and you have to write your own copy (and move if you want them) constructor and assignment (and move assignment) operators to make sure the object is copied and assigned correctly (See What is The Rule of Three? for more on why you need this stuff).
game::game() {
gameBoard = new board();
player_one = new player; //added to get player instance
player_two= new player; // added to get player instance
player_one->setPlayerNum(1);
player_two->setPlayerNum(2);
}
// new function: copy constructor
game::game(const game & source) {
gameBoard = new board(*source.gameBoard);
player_one = new player(*source.player_one);
player_two= new player(*source.player_two);
turns = source.turns;
maxTurns = source.maxTurns;
}
game::~game() {
delete gameBoard; // you needed to have this if you didn't already
delete player_one; // added to release player instance
delete player_two; //added to release player instance
}
// new function swap function (used by assignment operator)
game::swap(game & source) {
std::swap(gameBoard, source.gameBoard);
std::swap(player_one , source.player_one);
std::swap(player_two, source.player_two);
std::swap(turns, source.turns);
std::swap(maxTurns, source.maxTurns);
}
// new function assignment operator
game & operator=(game source)
{
swap(source);
return * this;
}
Look at all the extra stuff you have to write. Odds are there's a bug or two in there.
The assignment operator is taking advantage of the Copy and Swap Idiom. It's often not the fastest way to assign, but it is brutally easy to write and very hard to get wrong. I start with Copy and Swap and use it until life proves that it is a performance bottleneck.
If gameBoard
, player_one
and player_two
are Automatic variables, the compiler will generate the destructor and all of the copying code for you unless board
is poorly written. One of your goals should be to make all of your classes that own a resource observe the Rule of Three or Five so that classes that contain them can take advantage of the Rule of Zero.
Upvotes: 3