How to fix 'this was nullptr" when there was no call to "this"

Question

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;
}

player_one and player_two are not initialized in the ctor and yet you try to dereference them — Borgleader, Apr 20 '19 at 03:38
`playerNum` is also `this->playerNum`. It is a member variable, so it needs a valid `player` instance. — user4581301, Apr 20 '19 at 03:38

user4581301 · Accepted Answer · 2019-04-20T07:08:34.623

game::game() {
    gameBoard = new board();
    player_one->setPlayerNum(1);
    player_two->setPlayerNum(2);
}

does not instantiate any players 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){
}

Addendum

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.

@AriNiknia yes, but you greatly increase your workload by doing so. — user4581301, Apr 20 '19 at 07:09

How to fix 'this was nullptr" when there was no call to "this"

1 Answers1

Addendum