C++: Why is my vector of structs acting as one struct?

Question

I'm working my way through Accelerated C++ and have decided to mess around with the one of structs that were defined in there. While doing so, I've come across a problem: creating a vector of these structs and modifying the elements in each one seems to modify the elements in all of them.

I realize that this probably means I've initialized all the structs in the vector to a struct at a single memory address, but I used the .push_back() method to insert "dummy" structs in to the vector. I was under the impression that .push_back() pushes a copy of its argument, effectively creating a new struct.

Here is the header for the struct:

#ifndef _STUDENT_INFO__CHAPTER_9_H
#define _STUDENT_INFO__CHAPTER_9_H

#include <string>
#include <iostream>
#include <vector>

class Student_info9{
public:
    Student_info9(){homework = new std::vector<double>;};
    Student_info9(std::istream& is);

    std::string getName() const {return name;};
    double getMidterm() const {return midterm;};
    double getFinal() const {return final;};
    char getPassFail() const {return passFail;};

    std::vector<double> *getHw(){return homework;};

    void setName(std::string n) {name = n;};
    void setMidterm(double m) {midterm = m;};
    void setFinal(double f) {final = f;};


private:
    std::string name;
    double midterm;
    double final;
    char passFail;

    std::vector<double> *homework;
};


#endif  /* _STUDENT_INFO__CHAPTER_9_H */

And here is the code that i'm fooling around with (excuse the excessive print statements... the result of some time trying to debug :) ):

vector<Student_info9> did9, didnt9;

bool did_all_hw9(Student_info9& s)
{
    vector<double>::const_iterator beginCpy = s.getHw()->begin();
    vector<double>::const_iterator endCpy = s.getHw()->end();
    return(find(beginCpy, endCpy, 0) == s.getHw()->end());
}

void fill_did_and_didnt9(vector<Student_info9> allRecords)
{
    vector<Student_info9>::iterator firstDidnt = partition(allRecords.begin(), allRecords.end(), did_all_hw9);


    vector<Student_info9> didcpy(allRecords.begin(), firstDidnt);


    did9 = didcpy;

    vector<Student_info9> didntcpy(firstDidnt, allRecords.end());
    didnt9 = didntcpy;


}

int main(int argc, char** argv) {

    vector<Student_info9> students;

    Student_info9 record;

    for(int i = 0; i < 5; i++)
    {
        students.push_back(record);
    }

    for(int i = 0; i < students.size(); i++)
    {
        students[i].setMidterm(85);
        students[i].setFinal(90);

        students[i].getHw()->push_back(90);
        std::cout << "student[" << i << "]'s homework vector size is " << students[i].getHw()->size() << std::endl;
        students[i].getHw()->push_back(80);
        std::cout << "student[" << i << "]'s homework vector size is " << students[i].getHw()->size() << std::endl;
        students[i].getHw()->push_back(70);
        std::cout << "student[" << i << "]'s homework vector size is " << students[i].getHw()->size() << std::endl;

        std::cout << "Just pushed back students[" << i << "]'s homework grades" << std::endl;

        if(i == 3)
            students[i].getHw()->push_back(0);
    }

    std::cout << "student[3]'s homework vector size is " << students[3].getHw()->size() << std::endl;

    for(vector<double>::const_iterator it = students[3].getHw()->begin(); it != students[3].getHw()->end(); it++)
        std::cout << *it << " ";

    std::cout << std::endl;

    std::cout << "students[3] has " << ( ( find(students[3].getHw()->begin(),students[3].getHw()->end(), 0) != students[3].getHw()->end()) ? "atleast one " : "no " )
            << "homework with a grade of 0" << std::endl;

    fill_did_and_didnt9(students);


    std::cout << "did9's size is: " << did9.size() << std::endl;
    std::cout << "didnt9's size is: " << didnt9.size() << std::endl;

}

As you can see by the print statements, it seems that the homework grades are being added only to one Student_info9 object, copies of which seem to be populating the entire vector. I was under the impression that if you were to use consecutive copies of .push_back() on a single object, it would create copies of that object, each with different memory addresses.

I'm not sure if that's the source of the problem, but hopefully someone could point me in the right direction.

Thanks.

By the way, those header guards are [illegal](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). — GManNickG, Jan 20 '11 at 17:48

score 4 · Accepted Answer · answered Jan 20 '11 at 15:00

4

When you push a StudentInfo onto the vector, it is indeed copied, so that's not the problem. The problem is the vector containing the homework grades. Since you only store a pointer to that vector in StudentInfo, only the pointer, not the vector, is copied when you copy a StudentInfo. In other words you have many different StudentInfos that all have a pointer to the same homework vector.

To fix this you should define a copy constructor which takes care of copying the homework vector.

answered Jan 20 '11 at 15:00

sepp2k

363,768
54
674
675

Ok, instead of creating a copy constructor, I just dereferenced what was returned by the "new" statement and made the homework vector an actual vector (not a pointer to one). I also changed getHw() to return a vector instead of a pointer to one, and changed the appropriate "->" operators to "." operators in main(). Now the problem is that none of the homework grades are being added to the Student_info9 homework vectors! I thought the default copy constructor copied every field, so i'm at a loss as to why this does not work – Kevin Jan 20 '11 at 15:43
@Kevin: 1. You shouldn't use `new` if you don't want a pointer. 2. Unless you make the `getHw` method return a pointer or reference, it will return a copy of the vector. So if you do `students[i].getHw().push_back(0)` you're adding 0 to the copy returned by `getHw`, not to `student[i]`'s homework vector. – sepp2k Jan 20 '11 at 15:48
Ah, got it. One more question. Where is it implied that getHw() returns a copy of the homework vector, and not the homework vector itself? Is it because of the const keyword that applies to the calling object? – Kevin Jan 20 '11 at 16:24
@Kevin: No, it's because the return type is `vector`. In C++ the return value of a function is always copied unless the return type is a reference (i.e. `vector&`). – sepp2k Jan 20 '11 at 16:29

score 2 · Answer 2 · answered Jan 20 '11 at 14:58

2

Have you learned about the copy constructor yet? If so, think about what is happening with vector<Student_info9> students on push_back().

Specifically, what happens with this pointer.

   std::vector<double> *homework;

answered Jan 20 '11 at 14:58

Nim

33,299
2
62
101

score 1 · Answer 3 · answered Jan 20 '11 at 15:01

1

The line Student_info9 record; constructs a Student_info9 using the first constructor. This first constructor creates a vector and stores a pointer to it as a member variable. You then proceed to add a copy of this Student_info9 to a vector 5 times. Each copy has a pointer to the same vector.

answered Jan 20 '11 at 15:01

Null Set

5,374
24
37

Concise; the only reason this wasn't accepted as the answer is that there is no resolution... only a diagnosis. Great answer though. – Kevin Jan 20 '11 at 15:13

score 1 · Answer 4 · answered Jan 20 '11 at 15:01

Your StudentInfo9 class contanis a pointer to a std::vector<double>, which means in the default copy constructor (which will be called when you add a StudentInfo9 object to your vector), the pointer itself is copied. That means all of your StudentInfo9 objects have the same homework vector.

Does that make sense? Please refer to http://pages.cs.wisc.edu/~hasti/cs368/CppTutorial/NOTES/CLASSES-PTRS.html for a more in depth look at pointers and copy constructors.

C++: Why is my vector of structs acting as one struct?

4 Answers4