Ankur Lathwal
Ankur Lathwal

Reputation: 725

Add addresses of Objects to a vector in loop

I need to create several objects and put them in a list (for which I am using std::vector). Also, I need the list items to point to the addresses of the objects so that the changes I make to the objects are reflected in the list too. But the thing is, every item in the list is pointing to the last object created in the loop.

    for(int i=0;i<50;i++){
        for(int j=0;j<50;j++){
            Grass g1;
            g1.position.x = i;
            g1.position.y = j;
            grassList.push_back(&g1);
        }
    }

The the attributes of grass objects in the list should be..

[0,0]
[0,1]
[0,2]
.
.
.
.
[49,49]

But it's coming out to be..

[49,49]
[49,49]
[49,49]
[49,49]
.
.
.
[49,49]

Upvotes: 4

Views: 1779

Answers (3)

kfsone
kfsone

Reputation: 24269

(Modern-C++ update at the end)

If you're accustomed to other languages that use ref-counting of variables, you might have expected that

Grass g1;

was creating a new "Grass" object every iteration of the loop. It's not, C++ isn't a ref-counted language.

Instead, it creates a scoped local variable on the stack.

Because you're doing this in a loop, it's probably going to be at the same location in memory every time.

You will either need to:

  1. Instead of pointers, just make your container a container of Grass objects: let the container handle allocation for you.

  2. Use C++11's unique_ptr and C++14's make_unique to create an instance of Grass dynamically for each iteration of the loop. When the vector containing the unique_ptrs goes out of scope, they will be automatically freed.

  3. Use the new and delete keywords to manually allocate and release Grass objects to point to.

Option 1:

#include <vector>

struct Grass {
    struct {
        int x, y;
    } position;
};

int main() {
    std::vector<Grass> grassList;
    for(int i=0;i<50;i++){
        for(int j=0;j<50;j++){
            Grass g1;
            g1.position.x = i;
            g1.position.y = j;
            grassList.push_back(g1);
        }
    }  
}

Live demo: http://ideone.com/DQs3VA

Option 2:

#include <memory>
#include <vector>

struct Grass {
    struct {
        int x, y;
    } position;
};

int main() {
    std::vector<std::unique_ptr<Grass>> grassList;
    for(int i=0;i<50;i++){
        for(int j=0;j<50;j++){
            auto g1 = std::make_unique<Grass>();
            g1->position.x = i;
            g1->position.y = j;
            grassList.push_back(std::move(g1));
        }
    }   
}

Live demo: http://ideone.com/hJUdwR

Option 3:

#include <vector>

struct Grass {
    struct {
        int x, y;
    } position;
};

int main() {
    std::vector<Grass*> grassList;
    for(int i=0;i<50;i++){
        for(int j=0;j<50;j++){
            Grass* g1 = new Grass;
            g1->position.x = i;
            g1->position.y = j;
            grassList.push_back(g1);
        }
    }
    // ...
    for (auto& g1: grassList) {
        delete g1;
    }
    grassList.clear();
}

Live demo: http://ideone.com/GTk7ON


C++11 introduced emplace_back which lets you allocate and construct the entry in the container in one go.

#include <vector>

struct Grass {
    struct {
        int x, y;
    } position;

    // because x and y are inside a private sub-struct,
    // we'll need a constructor to pass the values in.
    Grass(int x, int y) : position{x, y} {}
};


int main() {
    std::vector<Grass> grassList{};    // default initialized.

    static constexpr size_t dim = 10;  // name the constant (DIMension)

    grassList.reserve(dim);            // allocate memory in advance

    for (auto i = 0; i < dim; i++) {
        for(auto j = 0; j < dim; j++) {
            grassList.emplace_back(i, j);
        }
    }

    // no cleanup required
}

live demo: https://gcc.godbolt.org/z/G1YsW7hMs

Upvotes: 5

If for whatever (unspecified) reason you need to store the pointers to the locations of the objects, you need this

std::vector<Grass*> grassList;  // Assumed you already have something like this
for(int i=0;i<50;i++){
    for(int j=0;j<50;j++){
        Grass* gp1 = new Grass;
        gp1->position.x = i;
        gp1->position.y = j;
        grassList.push_back(gp1);
    }
}

This avoids the destruction of the objects pointed at by the pointers you are storing. See, e.g., Creating an object: with or without `new` You will have to take care of suitably deleting objects with delete to free memory.

But depending on your needs, you may also use an array of objects instead of an array of pointers, as suggested by tuple_cat.

Upvotes: 0

flogram_dev
flogram_dev

Reputation: 42888

You're pushing pointers to local variables to the vector. Local variables get destroyed at the end of their scope (the second-to-last } in this case). Therefore, dereferencing any of those pointers after the } is undefined behavior. The output you're seeing is a completely valid result of undefined behavior.

Using pointers here just doesn't make sense. Use them (including new) only when absolutely necessary. For more info, see Why should I use a pointer rather than the object itself?

This is the right way:

std::vector<Grass> grassList;
         // ^^^^^ not pointers!

for(int i=0; i<50; ++i) {
    for(int j=0; j<50; ++j) {
        Grass g1;
        g1.position.x = i;
        g1.position.y = j;
        grassList.push_back(g1);
    }
}

Upvotes: 4

Related Questions