Object has members variables values changed after push operation to vector

Question

I have a std::vector<Item*> listOfItems which holds pointers to Item objects.

I have two parameters for the constructor of Item ,

Item::Item(std::string name, int amount)
{
    itemName = name;
    ownerAmount = amount;       
}

I also have Player objects, which have a member variable std::vector<Item> ownedItems.

I've created in my main file a vector with pointers to all the Item objects owned by the various Player declared as follow:

std::vector<Item*> itemList = listOfItems(players);

where

std::vector<Item*> listOfItems(std::vector<Player*> players) {
    std::vector<Item*> listOfItems itemList;
    for (int i = 0; i < players.size(); i++) {
        for (int j = 0; j < players.at(i)->getAllCountries()->size(); j++) {
            itemList.push_back(&players.at(i)->getAllCountries()->at(j));
        }
    }
    return itemList;
}

After this is declared, I'm running the strategy of my Player which is:

    player1->executeStrategy(&itemList);

Now in the doOperation (following the Strategy Pattern) of my player declared as follow:

    void HumanPlayerStrategy::doOperation(Item* x, std::vector<Item*> *itemList){
                    attackerVictory(x, itemList);   
}

I'm running the function attackerVictory with a pointer to the itemList declared in main with all the pointers to the various Item owned by the different Player(for the purpose of modifying these).

The only problem is, after I'm done doing operations in attackerVictory, any Item that is added to the itemList has blank member variable values. I can't seem to grasp why this would not work?

Here is my attackerVictory function:

void Strategy::attackerVictory(Item* x, std::vector<Item*> *listOfItems) {
    Item temp = *x;
    /* set different member variable values for the Item temp */
    listOfItems->push_back(&temp);
}

Doing so gives me am Item at the end if the listOfItems with empty itemName and ownerAmount values. What am I missing here?

Do **not** store pointers to internal vector items. If that vector is resized, those pointers you stored can become invalid. Example: `vector x; x.push_back(10); int *ptr = &x[0]; x.push_back(20);` That `ptr` may not point to valid memory if the vector `x` changes size. — PaulMcKenzie, Nov 21 '17 at 05:43
[Possible duplicate](https://stackoverflow.com/questions/6438086/iterator-invalidation-rules). — PaulMcKenzie, Nov 21 '17 at 05:47

score 0 · Accepted Answer · answered Nov 21 '17 at 05:50

0

You are adding reference to a temporary object which gets destroyed after the attackerVictory method is executed. It should be

void Strategy::attackerVictory(Item* x, std::vector<Item*> *listOfItems){
    Item * tempptr = new Item();
    *tempptr = *x;

    listOfItems->push_back(tempptr);
}

answered Nov 21 '17 at 05:50

Karthik Krish

139
6

1

A problem, but not the only problem. There is a pretty nasty architectural deficiency that needs to be resolved. Other than that, you can kill two birds with one stone. Take advantage of the copy constructor (`Item * tempptr = new Item(*x);`) rather than construct then assign. – user4581301 Nov 21 '17 at 06:00

Object has members variables values changed after push operation to vector

1 Answers1