Why can't I push_back an object into a vector of objects that belongs to another class?

Question

What I'm trying to do here is to add Voter to a vector that belongs to RegLogBook and, RegLogBook is an object that is owned by my Election class. I don't know what I'm doing wrong here, the Voter object is not being added to the vector. The relevant codes are below, I've removed all the unnecessary parts.

Election.h

class Election
{
public:
    Election();
    ~Election();
    RegLogBook getRegLogBook();
private:
    RegLogBook* _regLogBook;
};

Election.cpp

Election::Election()
{
    _regLogBook = new RegLogBook();
}

RegLogBook Election::getRegLogBook() {
    return *_regLogBook;
}

RegLogBook.h

class RegLogBook
{
public:
    RegLogBook();
    ~RegLogBook();
    vector<Voter*> getVoterList();

private:
    vector<Voter*> _voterList;
};

RegLogBook.cpp

vector<Voter*> RegLogBook::getVoterList() {
    return _voterList;
}

Committee.cpp register voter method

void Committee::RegVoter(RegLogBook logs) {
    Voter *newVoter = new Voter();
    logs.getVoterList().push_back(newVoter); //The voter is not added to the list
}

I call this in my main()

Election *Election2018 = new Election();
Committee com1 = new Committee();
com1.RegVoter(Election2018->getRegLogBook());

There must be a 1000 and a one duplicate, but I do not know how to look for them. In short, you are returning a **copy** of a member variable, and when you insert data in it, the original remains unchanged. — SergeyA, Nov 01 '18 at 15:18
offtopic: seems like you should have a `std::vector` rather than a `std::vector` — 463035818_is_not_an_ai, Nov 01 '18 at 15:21
Did you learn Java or C# previously? It's not good idea to draw parallels between C++ and those languages, because they work in different ways on basic level. I'd recommend [a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to learn — Yksisarvinen, Nov 01 '18 at 15:22
@SergeyA *but I do not know how to look for them* -- Probably because a lot of the questions start out as "why are my iterators invalid?", with the answer being that copies are returned from a `get...()` function instead of a reference. So it's probably hard to pinpoint a duplicate if the question starts out not sounding like the OP's. — PaulMcKenzie, Nov 01 '18 at 15:24
Are you learning C++ from a book? Since you don't appear to need polymorphic behavior, get rid of all those pointers (`Type*`) and dynamic allocations (`new Type`). — Chad, Nov 01 '18 at 15:32

Fureeish · Answer 1 · 2018-11-01T15:43:04.963

RegLogBook::getVoterList() returns your _voterList by value. That means it copies the whole vector and returns it. Then you add elements to that copy.

To fix this, simply change your

vector<Voter*> RegLogBook::getVoterList()

to

vector<Voter*>& RegLogBook::getVoterList()
//            ^ notice the reference part

There is another problem that I initially missed. In your Committee::RegVoter method, you take the argument by value. That means the method will be invoked with a copy of your RegLogBook. You should also change

void Committee::RegVoter(RegLogBook logs)

to

void Committee::RegVoter(RegLogBook& logs)
//      again notice the reference ^

_{Thanks to Lightness Races in Orbit for pointing that out}

Remember - to work on the original object, not a copy of it, you should pass it either by a reference or a pointer. By default you should prefer passing by reference, unless you have strong argument to use pointers

score 1 · Answer 2 · answered Nov 01 '18 at 15:20

1

I believe the problem is that you return the voter list by-value, meaning that a copy of the voter list is returned:

vector<Voter*> RegLogBook::getVoterList() {
    return _voterList;
}

So this line:

logs.getVoterList().push_back(newVoter);

will modify the local copy instead of the value being stored in the object instance.

Try modifying your code to return a reference (by changing the return type to be vector<Voter*>&)

vector<Voter*>& RegLogBook::getVoterList() {
    return _voterList;
}

or, maybe better, return a pointer:

vector<Voter*>* RegLogBook::getVoterList() {
    return &_voterList;
}

I prefer the last alternative, as this makes it more obvious that the returned value can be modified.

answered Nov 01 '18 at 15:20

Rulle

4,496
1
15
21

reference or pointer have the same meaning regarding the returned value can be modified, but pointers can be null which is the wrong thing unless the value returned actually can be `null` – 463035818_is_not_an_ai Nov 01 '18 at 15:29
Your last point is quite similar to claiming that you should prefer passing arguments by pointers rather than by reference, so it's obvious they are not copied. This is discouraged in every major C++ guideline. One should prefer the **first** solution here (*thanks **user463035818** for pointing out the typo in the original comment*) – Fureeish Nov 01 '18 at 15:34
Most of the time, mutating a return value seems like a bad idea and mutation, in general, is a source of many bugs. I believe this is the reason why the Google C++ style guide (https://google.github.io/styleguide/cppguide.html#Output_Parameters) discourages output parameters for functions, but if they have to be used, as pointer arguments (as opposed to a reference). This makes it clear at the call site that the argument is expected to be modified. By returning a pointer as opposed to a reference, it is also much more obvious that we are not modifying a local copy. – Rulle Nov 01 '18 at 15:46

score 0 · Answer 3 · edited Nov 01 '18 at 21:47

0

From what I'm seeing your code shouldn't even compile: you're using the new keyword, but you're saving its returned value into a Committee, not into a Committee* as you should. If you are using an overloaded new operator, please post the whole code.

edited Nov 01 '18 at 21:47

Fabio says Reinstate Monica

5,271
9
40
61

answered Nov 01 '18 at 16:02

Rokas Višinskas

533
4
12

Why can't I push_back an object into a vector of objects that belongs to another class?

3 Answers3