CODEWITHSUNDEEP
c++vectoriterator
VJC1288
VJC1288

Reputation: 87

How am I using vector iterators incorrectly?

This code works:

for (unsigned int i = 0; i != m_cResources.getLoadedPlayers().size(); i++) {
    m_cResources.getLoadedPlayers()[i]->update();
}

When I try to use iterators with the following code, my programs hangs and stops working:

for (std::vector<CPlayer*>::iterator i = m_cResources.getLoadedPlayers().begin();
        i != m_cResources.getLoadedPlayers().end(); i++) {
    (*i)->update();
}

getLoadedPlayers() returns the vector of CPlayer*. Update is a member function in CPlayer.

I've never used iterators before so I'm not really sure what is wrong. I don't think the error is outside of this code because the first block works. So I assume my implementation of the iterator is wrong. Any ideas?

EDIT: getLoadedPlayers() returns the vector by value, not reference. Would this most likely be the problem? But then wouldn't the first code block also not work? I will test this as soon as I can.

The update function modifies member values in a CPlayer based on flags previously set.

Upvotes: 1

Views: 380

Answers (3)

rems4e
rems4e

Reputation: 3172

Your vector is returned by value, so your iterator which is incremented each iteration of the loop will never "meet" the end iterator in the loop exit condition : it belongs to another vector (changed at each iteration). So your code invokes undefined behavior by dereferencing an invalid iterator (and the underlying pointer)

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 310930

If as you said getLoadedPlayers() returns the vector by value, not reference then the first loop also has no sense because in each iteration a new copy of the vector is used in expression

m_cResources.getLoadedPlayers()[i]->update();

When you use iterators in the loop

for (std::vector<CPlayer*>::iterator i = m_cResources.getLoadedPlayers().begin();
        i != .getLoadedPlayerm_cResourcess().end(); i++)

then iterator i and getLoadedPlayerm_cResourcess().end() point to differenent memory extent. So they may not be compared.

If you want that your loops had any sense you should use reference to the original vector.

Upvotes: 2

razeh
razeh

Reputation: 2765

Why not use C++'s for each?

for (auto &i : m_cResources.getLoadedPlayers()) {
  i->update();
}

Upvotes: 2

Related Questions