while creating and adding objects to a vector in a loop always adds same object

Question

I got a problem,

class student {
public:
    static int id,a,c;
    static bool editable;
    static std::queue <int> classes;
    static queue <int> classesT;
    static queue <int> classesD;
public:
    student(int x,int y,queue <int> z){
        editable=true;
        id=x;
        a=y;
        classes=z;
        c=classes.size();
    }

    void DisposeObject()
    {
       delete this;
    }
};

int main(){
std::vector <queue <int> >links;
std::vector <int> quotas;
std::vector <student> sList;
std::queue <int> classes;
std::queue <student> q1;
int a,c,sNum,cNum;
static int temp;
ifstream myFile("....");
if(myFile.is_open()){
    myFile>>sNum;
    myFile>>cNum;
}
for(int i=0;i<sNum;i++){
    myFile>>c;
    myFile>>a;
    for(int j=0;j<c;j++){
    myFile>>temp;
    classes.push(temp);
    }
    student *s1=new student(i,a,classes);
    sList.push_back(*s1);
    s1->DisposeObject();
    while(!classes.empty())
        classes.pop();
}

This is my code. I want to add different objects to my vector but whenever a new student created students that created before take its values and I end up with a vector something like:

a,a,a,a

instead of

a,b,c,d

By the way I already tried reserve function.

Why are you using new if the vector doesn't contain pointers? Can you show a complete example? — Retired Ninja
– Retired Ninja, Commented Dec 16, 2013 at 8:25
Are you sure you're reading different information from myFile during each iteration? If you end up with four instances of student in the vector, it means that the push_backs work but the information stored with the objects is incorrect. Also, why do you new the student? You can just as easily push_back a temporary and spare yourself the clean-up: sList.push_back(student(i,a,classes)); Also, does DisposeObject() commit sucicide, i.e. does it do delete this? Otherwise, you have a leak with every single student you new up. (Note: I don't recommend suicide!). — thokra
– thokra, Commented Dec 16, 2013 at 8:27
@user3106529 have you debugged your code? btw, where is the delete s1? — AndersK
– AndersK, Commented Dec 16, 2013 at 8:27
And is student copying classes? It seems like if you just declare vector<whatever> classes inside your for(int i...) loop, you'd get a fresh one each iteration. Your while() pop loop just seems either unnecessary or wrong. — Joe Z
– Joe Z, Commented Dec 16, 2013 at 8:40

Dietmar Kühl · Accepted Answer · 2013-12-16 09:32:22Z

1

The code isn't sufficient to pin-point exactly what the problem is. However, there are multiple things which are, at least, suspicious:

You don't check any of your inputs and just assume everything went well! My personal guess is that reading failed at the end of the first or the beginning of the second record and you just reuses the already read values. You always want to check your reads after you attempted to read, e.g.:
```
if (myFile >> a >> c) {
     // carry on
}
else {
    std::cout << "ERROR: failed to read record!\n";
    // recover, bail out, ...
}
```
In C++ you don't allocate objects on the heap unless you absolutely have to! Most of the time, you get away with some classes doing the allocation for you. I can't remember when I last used new in application code. Here is how you can append a student record:
```
sList.push_back(student(i, a, classes));
```
The object will, conveniently, clean-up after itself. That is, quickly, get rid of your DisposeObject() member function! C++ uses destructors and neatly do so because it isn't garbage collected. Do not try to carry over C# idioms to C++: it won't do you any good.
There is no need to remove every object individually from your vector. Just use clear():
```
classes.clear();
```

answered Dec 16, 2013 at 9:32

Dietmar Kühl

155k18 gold badges238 silver badges395 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Joe Z Over a year ago

I'd add that void DisposeObject() { delete this; } is a pretty bad idea. The restrictions you must adhere to to "get it right" are pretty onerous: parashift.com/c++-faq-lite/delete-this.html

Collectives™ on Stack Overflow

while creating and adding objects to a vector in a loop always adds same object

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related