Why won't the pointer store and print the same data as the object it is supposed to match? (C++)

Question

I am making a game where the user is allowed to choose a Player object from a vector of Player objects read in from a text file.

I ask the user to enter a name in order to choose a player from the vector and I iterate through the vector to look for a matching name (assume each player has a different name).

Once they match, I want to point my Player player1 pointer to that object in the player vector.

However, the player1 pointer is not printing the correct name outside of the for loop after it has found a match. The pointer instead defaults to printing the name of the last player in the text file every time rather than the name of the player that matches the input of the user.

What could I do to fix this issue ?

vector<Player> playerVectorReadIn;
Player *player1; string name="";

//reading in the data

ifstream inFile;

inFile.open("playerData.txt");
if(!inFile){
    cout<<"Error! Unable to open file";
    exit(1);
}
while(inFile>>name){
    Player playerObject(name);
    playerVectorReadIn.push_back(playerObject);
}
inFile.close();

//finding matching data in vector 

cout<<"\n\tEnter the name of the player from the list you choose: ";
getline(cin,name);

    for(Player p:playerVectorReadIn){
        if(p.getName()==name){
            flag=true;
            player1=&p; //setting pointer to player - https://stackoverflow.com/questions/2988273/c-pointer-to-objects
            cout<<name; //the user entered name (example Bob)
            cout<<p.getName(); //the matching name in the vector (Bob)
            //both print the same name here so it works
        }
    }

if(flag==true){
    cout<<"\nYou will be playing as: "<<player1->getName(); 
    //prints as the name of the last object in the text file (example Ryan)
    //not the matching name as above - why?
}else{
    cout<<"\nPlayer not found.";
}
flag=false; name="";

Text file contents example :

Dave 
Jill 
Bob 
Mary 
Donna
Ryan

score 1 · Accepted Answer · answered May 09 '20 at 16:22

for(Player p:playerVectorReadIn){

This is iterating by value, p is an object that's local in scope to this for loop, and is a copy of the object in the vector. Each time this loop iterates this object gets destroyed. If the loop iterates again, a new p object gets created.

     player1=&p;

This saves a pointer to this local object. However, as we've just discovered, p gets destroyed at the end of the loop, player1 becomes a dangling pointer to a destroyed object, and using it subsequently becomes undefined behavior.

This explains the garbage results you're observing. Fortunately, the solution is very simple, iterate by reference:

for(Player &p:playerVectorReadIn){

p is now a reference to the actual object in the vector, and as long as the vector does not itself get subsequently reallocated, the pointer to the object in the vector will remain valid.

@magma99 take note of the last sentence: "_the pointer to the object in the vector will remain valid as long as the vector does not itself get subsequently reallocated_"; taking a ponter/reference to an object included in a vector is usually a bad idea. If you need an object also outside the vector, consider using a `vector>` — Gian Paolo, May 09 '20 at 20:15

score 0 · Answer 2 · answered May 09 '20 at 16:22

In this loop:

for(Player p : playerVectorReadIn){
        if(p.getName()==name){
            player1=&p; //setting pointer to player

you are making a copy of the Player and then taking it's address. The copy will die at the end of the loop, and you are left with a dangling pointer.

You need to iterate through the vector by reference:

for(Player & p:playerVectorReadIn){
        if(p.getName()==name){
            player1=&p; //setting pointer to player (correctly)

Why won't the pointer store and print the same data as the object it is supposed to match? (C++)

Text file contents example :

2 Answers2