A struct is not updating one of its member variables in c++

Question

I have a struct Creature and a struct Game. Game is a "friend" of Creature. In game I have vector creatures; and I add a creature x to that vector thourgh a function called addC

void addc (Creature& c){
    creatures.push_back(c);
}

Now I'm in another function "foo" that is a public method of the struct Game.

void foo (Creature& c){
    ...
}

In that function I need to find another creature from the vector creatures that matches some information from Creature c. So I made another public method in Game called fooHelper

void fooHelper (char s, int x, int y){
    bool found = false;
    for (int i = 0; i < creatures.size() && (!found); ++i){
        Creature& c = creatures[i];
        if (x == c.x && y == c.y){
            c.s = s;
            found = true;
        }
    }
}

however when I check if the second creature's "s" member is being updated, it turns out that it is not! I don't understand what I'm doing wrong since I'm pushing by references to the vector. and I'm getting the creature by reference from the vector.

the vector in game looks like this

struct Game{
    private:
        vector<Creature> creatures;
    ...
}

struct Creature{
    private:
        char s;
        int x; int y;
    ...
}

any help would be much appreciated!

Andy Prowl · Accepted Answer · 2013-04-02T23:05:28.653

This statement:

creatures.push_back(c);

Stores a copy of c into your vector: standard containers have value semantics. If you need reference semantics, you should store pointers into your vector.

Usually it is a good idea to use smart pointers, and which one to use depends on the ownership policy of your application. In this case, based on the information I could get from your question's text, it seems reasonable to let Game be the unique owner of all Creatures in the game (and therefore the only object which is responsible for the lifetime of the owned Creatures, and in particular for destroying them when they won't be needed anymore), so std::unique_ptr should be a good choice:

#include <memory> // For std::unique_ptr

struct Game{
private:
    std::vector<std::unique_ptr<Creature>> creatures;
    ...
};

Your member function addc() would then become:

void addc(std::unique_ptr<Creature> c)
{
    creatures.push_back(std::move(c));
}

And a client would invoke it this way:

Game g;
// ...
std::unique_ptr<Creature> c(new Creature());
g.addc(std::move(c));

Your foohelper() function, on the other hand, would be rewritten into something like this:

void fooHelper (char s, int x, int y) {
    bool found = false;
    for (int i = 0; i < creatures.size() && (!found); ++i){
        std::unique_ptr<Creature>& c = creatures[i];
        if (x == c->x && y == c->y) {
            c->s = s;
            found = true;
        }
    }
}

Finally, your class Game could return non-owning raw pointers (or references) to clients requiring access to the stored creatures.

score 1 · Answer 2 · edited May 23 '17 at 12:21

When you push your creature reference into the vector, it's making a copy. It's a vector of type "Creature", and so it's making a copy from the reference that you give it. One solution would be to keep a vector of creature pointers.

edit - this question helps explain things a little better than I was able to on why you can't have a vector of references: Why can't I make a vector of references?

A struct is not updating one of its member variables in c++

2 Answers2