C++- Reading from text file into vector bug

Question

I'm trying to read from Scores.txt into my Player vector. using the following code.

std::ifstream fin;
std::string alo;
int al;
Player* p = new Player;
std::vector<Player*> mPlayer;
fin.open("Scores.txt");
while (fin.good()) {
    fin >> alo >> al;
    p->setName(alo);
    p->setScore(al);
    mPlayer.push_back(p);
}

my text file is as follows:

Ali        25
Reza       101
Igor        18
Katie       20
Jacky        18
macky        20

however, after outputting myPlayer vector I get the following result:

macky        20
macky        20
macky        20
macky        20
macky        20
macky        20

you are using the same pointer(and data comming with it) to initaliaze all elements of your vector — Tyker, Jul 29 '18 at 16:12
You are pushing a pointer to the same Player instance into the vector again and again. You need to create a new instance per iteration. Also worth to read: https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong — πάντα ῥεῖ, Jul 29 '18 at 16:12
See also https://en.cppreference.com/w/cpp/language/operators scroll down to Stream extraction and insertion. Using std::vector would let the standard library handle your memory management. — Kenny Ostrom, Jul 29 '18 at 16:51
Unrelated: Consider replacing `while (fin.good()) { fin >> alo >> al;` with `while (fin >> alo >> al) {`. The first checks that the previous read was successful and then enters the loop and uses the results of the next read without testing for success. The second reads and tests, so the read values are only used if the read succeeded. ; — user4581301, Jul 29 '18 at 16:52
and after reading those links, see https://stackoverflow.com/questions/16727125/how-does-stdcopy-work-with-stream-iterators — Kenny Ostrom, Jul 29 '18 at 17:02
See [why `eof()` in a loop condition is wrong](https://stackoverflow.com/q/5605125/9254539). You have the same problem. — eesiraed, Jul 29 '18 at 17:10

score 2 · Answer 1 · answered Jul 29 '18 at 17:00

You are setting the same Player instance all the time. You need to create each time new Player and set their resources. This could be done:

while (fin.good()) {
    Player* p = new Player;  // -------> here
    fin >> alo >> al;
    p->setName(alo);
    p->setScore(al);
    mPlayer.push_back(p);
}

However, you could have used at best either simply std::vector<Player> mPlayer as it also will be created at heap

or

If you really need pointers, could have used std::vector<std::unique_ptr<Player>> mPlayer rather than the raw pointers (read about smart pointers here).

@LinusHaber Then you can also accept my answer ;) [See this](https://meta.stackexchange.com/questions/5234/how-does-accepting-an-answer-work) — Const, Aug 18 '18 at 02:31

score 1 · Answer 2 · answered Jul 29 '18 at 17:26

I just wrote a working version for your problem. The most important difference is the allocation of several Players inside the loop. Here a working version of your code:

#include "stdafx.h"
#include <fstream>
#include <iostream>
#include <sstream>
#include <iomanip>
#include <vector>

class Player
{
private:
    std::string name;
    unsigned int score;
public:
    void setName(std::string tosetName)
    {
        name = tosetName;
    }
    void setScore(unsigned int tosetScore)
    {
        score = tosetScore;
    }
};

int main()
{
    std::ifstream fin;
    std::string alo;
    int al;
    std::vector<Player*> mPlayer;
    fin.open("Scores.txt");
    while (fin.good()) 
    {
        Player* p = new Player;
        fin >> alo >> al;
        p->setName(alo);
        p->setScore(al);
        mPlayer.push_back(p);

    }   
    return 0;
}

C++- Reading from text file into vector bug

2 Answers2