push_back getting a vector of last element by self created object

Question

I'm trying to read in a list of Animals, this works fine. Then I want to split each string into two substrings for the name and cmc, this also works fine. But my cout doesn't work.

For example, my animal.txt is:

Dog|hi 
cat|miau
cow|hihi

My output of the for loop should look like this:

Dog
cat
cow

But the actual output is:

cow
cow
cow

Here is my Animal.cpp:

#include <string>;
#include <vector>;
#include <fstream>;
#include "Animal.h"

using namespace std;

string cmc;
string name;

void Animal();

void Animal(string nameA) {
    name = nameA;
}

void Animal(string nameA, string cmcValue) {
    name = nameCard;
    cmc = cmcValue;
}

void Animal::setName(string names)
{
    name = names;
}

void Animal::setCmc(string cmcvalue) {
    cmc = cmcvalue;
}

std::string Animal::getName() {
    return name;
}

std::string Animal::getCmc() {
    return cmc;
}

void Animal::openfileAnimal() {

    ifstream inFileAnimal;

    inFileAnimal.open("Animals.txt");

    if (inFileAnimal.fail()) {
        cerr << "error open this file" << endl;
        exit(1);
    }

    string itemsAnimal;
    std::vector<Animal> AllAnimals;

    while (getline(inFileAnimal, itemsAnimal)) {
        Animal c;
        string t1 = itemAnimal;
        size_t pos = t1.find("|");
        //name (setname(sub))
        string sub = t1.substr(0, pos);
        c.setName(sub);

        string t2 = t1.substr(sub1.length() + 1, t1.length());
        string sub2 = t2.substr(0, t2.length());

        c.setCmc(sub2);

        AllAnimals.push_back(c);
    }
    for (int i = 0; i < 2; i++) {
        std::cout <<AllAnimals.at(i).getName() << endl;
    }
}

I read nother StackOverflow questions like mine, but for my example all the solutions don't work. So where is my problem? I guess it's something like I am modifying the same memory over and over.

Why do you have two searches for the `|` in your loop? Each line contains only one `|` character. — PaulMcKenzie, Jun 17 '20 at 22:31
...Those methods aren't constructors, and those objects aren't members of the class.... — Mooing Duck, Jun 17 '20 at 22:48

Dmitry Kuzminov · Answer 1 · 2020-06-17T23:14:59.200

You have global variables instead of class data members:

string cmc;
string name;

void Animal();

void Animal(string nameA) {
    name = nameA;
}

void Animal(string nameA, string cmcValue) {
    name = nameCard;

    cmc = cmcValue;
}

As a result, you keep only the latest assigned values. Moreover, something that you probably treat as a constructor, is not a constructor at all. The constructor should look like:

Animal::Animal(string nameA, string cmcValue)
  : name(nameA), cmc(cmcValue) {
}

Note that initialization list syntax: that allows to avoid the mistakes like your's.

By the way, in your code nameCard is not defined at all.

push_back getting a vector of last element by self created object

1 Answers1