Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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]
like image 474
Ankur Lathwal Avatar asked Oct 15 '25 14:10

Ankur Lathwal


2 Answers

(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

like image 153
kfsone Avatar answered Oct 17 '25 03:10

kfsone


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);
    }
}
like image 21
emlai Avatar answered Oct 17 '25 02:10

emlai