0

Lets suppose i have a class Player which has some meber function. One of them gives back the pointer of the object using the this keyword. Player* getPlayer() { return this;};

class Player
{
public:

    Player(int id, string name, int score = 0) : pId(id), pName(name), pScore(score) { };
    void rollDice();

    int getId() { return pId; }
    int getScore() { return pScore; }
    string getName() { return pName; }
    Player* getPlayer() { return this;};
private:
    int pId;
    std::string pName;
    int pScore;
    unsigned char playerDiceRolled;
    Dice mDice;
};

I also have a class called Game which saves the information of any player in different vectors by using a function. One of this vectors saves the pointer of the object vector playerList;

class Game
{
public:


    void Winer();
    void addPlayer(Player A) ;
    void updateScore();
    vector<Player*> playerList;
    vector<int> idList;
    vector<string> nameList;
    vector<size_t> scoreList;

};

void Game::addPlayer(Player A)
{

    this->playerList.push_back(A.getPlayer());
    this->idList.push_back(A.getId());
    this->nameList.push_back(A.getName());
    this->scoreList.push_back(A.getScore());

}

My question is can i take the pointer out of that vector and use it to call a function of that class and return the wright value.

i tried that but it doen't work

I can see that the other values of the Player are saved correctly but when i call a function using the pointer to get a value it returns null. For example

int main()
{
    Game Game1;
    Player A(0, "Player1");
    Player B(1, "Player2");
    Game1.addPlayer(A);

    Game1.addPlayer(B);

    cout << "Name is:" << Game1.nameList[1] << "\n";



    cout << "Name is:" << Game1.playerList[1]->getName() << "\n";






    return 0;

}

The cout gives the wright name of player for Game1.nameList[1] but NULL value for Game1.playerList[1]->getName()

Aris
  • 138
  • 1
  • 12
  • Possible duplicate of [C++ taking address of temporary when pushing back a pointer to a vector of pointers](https://stackoverflow.com/questions/8270125/c-taking-address-of-temporary-when-pushing-back-a-pointer-to-a-vector-of-point) – Alan Birtles Feb 28 '19 at 09:35
  • `addPlayer(Player A)` takes a copy of the Player object. And that copy of `A` is destructed at the end of the method `addPlayer`. With `this->playerList.push_back(A.getPlayer());` you save the pointer to a temporary object in the vector. – t.niese Feb 28 '19 at 09:36
  • Instead of getPlayer you can just use the ref operator `&`. And you could just pass the Pointer to the object directly. And then you can use a pattern known was smart pointers to make sure you don't destroy in-use objects. Have a look at [unique_pr](https://en.cppreference.com/w/cpp/memory/unique_ptr) and [shared_ptr](https://en.cppreference.com/w/cpp/memory/shared_ptr). – bitmask Feb 28 '19 at 09:41
  • `Player::getPlayer` is *entirely* superfluous. If you have a pointer to a `Player`, you already have the result of `getPlayer`. If you have a reference to a `Player`, there is already `&` or `std::addressof` that do the same thing – Caleth Feb 28 '19 at 09:50

2 Answers2

4

That's because you are storing a pointer to a copy of your object:

void Game::addPlayer(Player A)

You need to use a reference and store it:

void Game::addPlayer(Player& A)

Otherwise you get an undefined behavior.

Matthieu Brucher
  • 21,634
  • 7
  • 38
  • 62
  • 1
    Yes, if the object pointed (and not a copy) is changed. That's the advantage of pointers! – Matthieu Brucher Feb 28 '19 at 10:24
  • if i got it wright the problem in the first case was that when i was passing the object to the function i was creating a copy to pass it which had a different pointer from the object that i wanted and that was being destructed at the end of the function wright? – Aris Feb 28 '19 at 10:52
1

addPlayer(Player A) takes a copy of the Player object. And that copy of A is destructed at the end of the method addPlayer. With this->playerList.push_back(A.getPlayer()); you save the pointer to a temporary object in the vector.

You should think over your code design in general. The game has list of Players so the game has either a shared or an exclusive ownership of those Player objects. And because of that you should not use a raw pointer here.

If you have an exclusive ownership you would use:

std::vector<std::unique_ptr<Player>> playerList;

Or

std::vector<Player> playerList;

For a shared ownership you would use:

std::vector<std::shared_ptr<Player>> playerList;

So a setup could look like this:

int main()
{
    Game Game1;

    Game1.addPlayer(std::move(std::make_unique<Player>(0, "Player1"));
    Game1.addPlayer(std::move(std::make_unique<Player>(0, "Player2"));

    cout << "Name is:" << Game1.nameList[1] << "\n";
    cout << "Name is:" << Game1.playerList[1]->getName() << "\n";

    return 0;
}


class Game {
public:
    void Winer();
    void addPlayer(Player A);
    void updateScore();
    vector<std::unique_ptr<Player>> playerList;
    vector<int> idList;
    vector<string> nameList;
    vector<size_t> scoreList;
};

void Game::addPlayer(std::unique_ptr<Player> A)
{
    this->idList.push_back(A->getId());
    this->nameList.push_back(A->getName());
    this->scoreList.push_back(A->getScore());

    // adding A to the vector has to be the last action, because A is moved to the vector
    this->playerList.push_back(std::move(A));
}

Raw pointer are now bad in general, but you don't use them when you want to express ownership. So if you pass a Player object to a function that should not take ownership of that object, then you would pass it as raw pointer:

void print_player_info(Player * const p) {
    // only prints the player info so no ownership is taken
    // p is only use within print_player_info  
    cout << "Name is:" << p->getName() << "\n";
} 


int main()
{
    Game Game1;

    Game1.addPlayer(std::move(std::make_unique<Player>(0, "Player1"));
    Game1.addPlayer(std::move(std::make_unique<Player>(0, "Player2"));

    print_player_info(Game1.playerList[1]->get());

    return 0;
}
t.niese
  • 39,256
  • 9
  • 74
  • 101