-1

I am reading objects from a text file and placing them in a vector. I am then trying to display them using a vector. I have the program working but not quite correctly. The problem I am having is the vector a collection of pointers and the objects are being overwritten and I am only getting the last two objects of the text file. Any idea on how to fix this so I can get all of the information from the text file not just the last two entries?

entities.txt

4
hero
Fred The Strong
100
The Outcast
30.89
22
18
9
6
villain
Scout
25
Goblin
3
0
villain
Arch Shaman
50
Orc
60
1
hero
Countess de Winter
100
none
200.00
9
11
19
14

main

#include "Entity.h"
#include "Hero.h"
#include "Villian.h"
#include <cstdlib>
#include <sstream>
#include <fstream>
#include <vector>

using namespace std;


int main(int argc, char** argv) {
    vector<Entity *> fileObjects;
    Hero hero1;
    Villian villian1;
ifstream readFile("entities.txt");
string line;
getline(readFile, line);
while(!readFile.eof())
{
    getline(readFile, line);
    if(line == "hero")
    {
        getline(readFile, line);
        hero1.name = line;
        getline(readFile, line);
        hero1.hp = stoi(line);
        getline(readFile, line);
        hero1.guild = line;
        getline(readFile, line);
        hero1.coin = stof(line);
        getline(readFile, line);
        hero1.strength = stoi(line);
        getline(readFile, line);
        hero1.dexterity = stoi(line);
        getline(readFile, line);
        hero1.faith = stoi(line);
        getline(readFile, line);
        hero1.wisdom = stoi(line);
        fileObjects.push_back(&hero1);
    }
    else if (line == "villain")
    {
        getline(readFile, line);
        villian1.name = line;
        getline(readFile, line);
        villian1.hp = stoi(line);
        getline(readFile, line);
        villian1.race = line;
        getline(readFile, line);
        villian1.level = stoi(line);
        getline(readFile, line);
        if(line == "0")
        {
            villian1.isBoss = false;
        }
        else
        {
            villian1.isBoss = true;
        }
        fileObjects.push_back(&villian1);
    }
    
}
readFile.close();

for(int i = 0; i < fileObjects.size(); i++)
{
    fileObjects[i]->display();
}
    return 0;
}

Entity header

#include <iostream>

using namespace std;

#ifndef ENTITY_H
#define ENTITY_H

class Entity {
public:
    string name;
    int hp;
    virtual void display();
    Entity(string name, int hp);
    Entity();
    Entity(const Entity& orig);
    virtual ~Entity();
private:

};

#endif /* ENTITY_H */

Entity source


#include "Entity.h"

Entity::Entity() {
    name = "";
    hp = 0;
}
Entity::Entity(string name, int hp)
{
    this->name = name;
    this->hp =hp;
}
void Entity::display()
{
    cout << "Name: " << name << endl;
    cout << "HP: " << hp << endl;
}
Entity::Entity(const Entity& orig) {
}

Entity::~Entity() {
}

Hero header

#include "Entity.h"

#ifndef HERO_H
#define HERO_H

class Hero:public Entity {
public:
    string guild;
    float coin;
    int strength;
    int dexterity;
    int faith;
    int wisdom;
    void display();
    Hero(string name, int hp, string guild, float coin, int strength, int dexterity, int faith, int wisdom);
    Hero();
    Hero(const Hero& orig);
    virtual ~Hero();
private:

};

#endif /* HERO_H */

Hero source

#include "Hero.h"

Hero::Hero() {
    guild = "";
    coin = 0.0f;
    strength = 0;
    dexterity = 0;
    faith = 0;
    wisdom = 0;
}
Hero::Hero(string name, int hp, string guild, float coin, int strength, int dexterity, int faith, int wisdom):Entity(name, hp)
{
    this->guild = guild;
    this->coin = coin;
    this->strength = strength;
    this->dexterity = dexterity;
    this->faith = faith;
    this->wisdom = wisdom;
}
void Hero::display()
{
    Entity::display();
    cout << "Guild: " << guild << endl;
    cout << "Coin: " << coin << endl;
    cout << "Strength: "  << strength << endl;
    cout << "Dexterity: " << dexterity << endl;
    cout << "Faith: " << faith << endl;
    cout << "Wisdom: " << wisdom << endl;
}
Hero::Hero(const Hero& orig) {
}

Hero::~Hero() {
}

Villian header

#include "Entity.h"

#ifndef VILLIAN_H
#define VILLIAN_H

class Villian:public Entity {
public:
    string race;
    int level;
    bool isBoss;
    void display();
    Villian(string name, int hp, string race, int level, bool isBoss);
    Villian();
    Villian(const Villian& orig);
    virtual ~Villian();
private:

};

#endif /* VILLIAN_H */

Villian source

#include "Villian.h"

Villian::Villian() {
    race = "";
    level = 0;
    isBoss = false;
}
Villian::Villian(string name, int hp, string race, int level, bool isBoss):Entity(name, hp)
{
    this->race = race;
    this->level = level;
    this->isBoss = isBoss;
}
void Villian::display()
{
    Entity::display();
    cout << "Race: " << race << endl;
    cout << "Level: " << level << endl;
    cout << "Is A Boss: "  << isBoss << endl;
}
Villian::Villian(const Villian& orig) {
}

Villian::~Villian() {
}

  • 3
    Please extract a [mcve] from your code. It should be just a single file. Also, state exactly the expected and actual results. As a new user here, also take the [tour] and read [ask]. BTW: I guess you mean "villain", not "villian". – Ulrich Eckhardt Aug 17 '21 at 16:20
  • 1
    There seems to be confusion regarding _what pointers are_. Your program has exactly one `Hero` variable and one `Villian` variable, and you keep pushing back the same pointers to one of those two variables. – Drew Dormann Aug 17 '21 at 16:21
  • This is a minimal reproducible example. You have everything you need to run the code. – johnnyboy1999 Aug 17 '21 at 16:21
  • I understand what pointers are...I am just trying to pull the data from the text file into the vector and display it. – johnnyboy1999 Aug 17 '21 at 16:23
  • This format is error prone and does not give your parser any fault points before it fails horribly. – Captain Giraffe Aug 17 '21 at 16:23
  • FWIW, You can eliminate the `this->` syntax by using a naming convention that has different names between members and parameters. For example, some coding styles prefix member names with "m_". – Thomas Matthews Aug 17 '21 at 16:48
  • Unrelated reading that should help with a future bug: [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – user4581301 Aug 17 '21 at 17:24

1 Answers1

1

The problem is that you are only pushing the same pointer to your Hero/Villain object onto your vector and you are overwriting the original object. Thus, when you print out the contents you are only ever printing out the same memory contents (which are being re-written as you read the entities file.)

Instead, you should either allocate a new Hero/Villain object on the heap (e.g. using new, or by creating a shared_ptr or unique_ptr.)

As a keen commenter points out, you do not want to push new Entity by value since you're relying on the polymorphic behavior of the Entity class and its derived classes [1].

1 - https://en.wikipedia.org/wiki/Object_slicing

Tom
  • 18,685
  • 15
  • 71
  • 81
  • 1
    Careful! Pushing `Entity` by value will [slice the objects](https://stackoverflow.com/questions/274626/what-is-object-slicing) – Drew Dormann Aug 17 '21 at 16:25
  • Ok I tried making a new Villian object on the heap but it still just gave me the last entry. – johnnyboy1999 Aug 17 '21 at 16:27
  • Yeah I need everything. – johnnyboy1999 Aug 17 '21 at 16:28
  • Any object you push to the vector will need to be a new heap object in this case. Thus, you could push a `new Hero(...)` if you read a hero and `new Villan(...)` if you read a villain. – Tom Aug 17 '21 at 16:31
  • So if I am trying to create a new Hero and new Villian object on the Heap. Any more clarification on how I do this? I mean I know how to do that but where? Because I tried before my while loop and it still does the same thing. Do I need to put it in the loop? – johnnyboy1999 Aug 17 '21 at 16:34
  • Yes, in the loop. – Tom Aug 17 '21 at 16:36
  • @AlanBirtles I'm not sure what you mean. At the top of the "main" code segment, there are two local objects declared. The OP is pushing the address of each of these to the vector. Although it's difficult to tell this because the formatting is inconsistent, it's not clear that these go out of scope or are replaced by other objects at any point. – Tom Aug 17 '21 at 17:08