Vector of pointer points to NULL when using push_back()

Question

I have the following struct in my code which assigns x and y randommly generated integer values:

struct A{
    int x,y;
    A () : x(random_gen_num),y(random_gen_num) {};   
};

I have a vector of objects and need a vector of pointers pointing to each object:

vector<A> a(5);
a.reserve(5);

vector<const A*> apoint(5);
apoint.reserve(5);
for(const A thisA : a){
    apoint.push_back(&thisA);
}

When I try printing them using:

for(unsigned int i = 0; i<5; i++){
    cout<< i <<"\t"<< a[i].x <<"\t" << &a[i]<<"\t" << apoint[i] <<endl;
}

I get all my apoint as NULL pointers as follows:

0       8       0x29b1bd0       0
1       8       0x29b1bd8       0
2       1       0x29b1be0       0
3       8       0x29b1be8       0
4       6       0x29b1bf0       0

The code seems logical to me and works when I say apoint[i] = &a[i] in the for loop, but in the real case this is not valid as the indices of apoint and a might not be same.To my knowledge, I couldn't find an answered question with a similar problem. Is there a better way to insert pointers into vectors?

What is the actual problem you're trying to solve? Why do you think you need a vector of pointers to objects in another vector? — David Schwartz, Jun 17 '19 at 18:58
yes, not clear why you need `apoint`. Simply use `&a[i]` when you want the address of an element in `a`, though I doubt that you even need that — 463035818_is_not_an_ai, Jun 17 '19 at 18:59
Yes, a vector of pointer is needed because in the actual program I want to that to another function for evaluation. But, thanks. I did not think about the scope of the thisA. — Akki, Jun 17 '19 at 19:04
`apoint` contains ten pointers – five null pointers followed by five dangling pointers. — molbdnilo, Jun 17 '19 at 19:05

score 1 · Answer 1 · answered Jun 17 '19 at 18:57

1

for(const A thisA : a){
    apoint.push_back(&thisA);
}

You push a pointer that is no longer valid on next loop. You need a reference (A&).

All that said of course, assuming you really need a vector of pointers (do you?)

answered Jun 17 '19 at 18:57

Michael Chourdakis

10,345
3
42
78

score 1 · Accepted Answer · answered Jun 17 '19 at 19:51

vector<const A*> apoint(5);

Makes a vector and preloads it with 5 const A*s that will be default initialized. That means apoint, before you push_back anything contains 5 null pointers. Since the size is already 5, the apoint.reserve(5); does effectively nothing. This also applies to vector<A> a(5);.

This means

for(unsigned int i = 0; i<5; i++){
    cout<< i <<"\t"<< a[i].x <<"\t" << &a[i]<<"\t" << apoint[i] <<endl;
}

prints out those 5 null pointers before it will reach the five added with push_back. Change

vector<const A*> apoint(5);

to

vector<const A*> apoint;

to construct an empty vector or assign into those 5 pointers with apoint[index] = &thisA; and remove the Theapoint.reserve(5);`.

But...

for(const A thisA : a){
    apoint.push_back(&thisA);
}

Needs to be broken down a bit more to spot a second bug. const A thisA : a makes copies of the elements in a that only exist for a single iteration of the for loop. These pointers are too short-lived to be of any use to you. You want references to the values in a, not copies, so

for(const A thisA : a){
    apoint.push_back(&thisA);
}

needs to be

for(const A & thisA : a){
    apoint.push_back(&thisA);
}

A note about storing references to items in vectors: This is usually a bad idea. If ask a new question and explain your goal, we may be able to suggest better alternatives. If you really do need pointers to vector elements, be very careful that you do not violate the Iterator invalidation rules .

Vector of pointer points to NULL when using push_back()

2 Answers2