Sign in to follow this  

Iterator is not Incremental

This topic is 1991 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi all,

I am trying to carry out some housekeeping on my game by checking for when an object (a bullet in this case) leaves the screen area and deleting that object.

[code]
void checkBullets()
{
int bulletIterator = -1;
for(std::vector<bullet>::iterator IT = aBullet.begin(); IT != aBullet.end(); IT++)
{
(*IT).move();
bulletIterator++;
if((*IT).xPosition() > SCREEN_WIDTH)
{
aBullet.erase(aBullet.begin()+bulletIterator);
}
}
}
[/code]

Unfortunately, when the bullet reaches the edge of the screen, the game freezes and I am given an assert error and the message is:

Vector iterators not Incremental

Can anyone see what I am doing wrong? I use the same erase line in another part of the program to delete bullets when they hit an enemy and this seems to work fine.

Thanks

Share this post


Link to post
Share on other sites
When you erase something in a vector it invalidates ALL iterators into that vector as elements can move around, memory can change etc so it is no longer valid.

The correct way to do this, via the for-loop method would be

[CODE]
for(std::vector<bullet>::iterator it = aBullet.begin(); it != aBullet.end(); ;)
{
*it.move();
if(*it.xPosition() > SCREEN_WIDTH)
{
it = aBullet.erase(it);
}
else
{
++it;
}
}
[/CODE]

vector.erase() returns the next valid iterator after the one you are erasing.
Note the pre-inc on the iterator - you should prefer this in most cases as it removes copies and is generally more efficient.

(This isn't the best example of how to do this however, there are better ways to code the loop/removal but I'm pressed for time).

Share this post


Link to post
Share on other sites
[quote]
(This isn't the best example of how to do this however, there are better ways to code the loop/removal but I'm pressed for time).
[/quote]

The C++ standard library provides some functionality for helping us do this - the so called "remove erase idiom":
[code]
#include <algorithm>

bool isBulletOffScreen(const Bullet &bullet)
{
return bullet.xPosition() > SCREEN_WIDTH;
}

void checkBullets()
{
std::vector<Bullet>::iterator i = std::remove_if(bullets.begin(), bullets.end(), &isBulletOffScreen);
bullets.erase(i, bullets.end());
}
[/code]

We can use a functor instead:
[code]
#include <algorithm>

struct IsBulletOffScreen
{
bool operator()(const Bullet &bullet) const
{
return bullet.xPosition() > SCREEN_WIDTH;
}
};

void checkBullets()
{
std::vector<Bullet>::iterator i = std::remove_if(bullets.begin(), bullets.end(), IsBulletOffScreen());
bullets.erase(i, bullets.end());
}
[/code]
This is slightly more inliner friendly, but at the expense of a little boiler plate.

In C++Ox, that can become:
[code]
#include <algorithm>

void checkBullets()
{
auto i = std::remove_if(bullets.begin(), bullets.end(), [](const Bullet &bullet) {
return bullet.xPosition() > SCREEN_WIDTH;
});
bullets.erase(i, bullets.end());
}
[/code]
We can use the lambda to eliminate the named function (which might not be used for any other purpose). In addition, the auto keyword simplifies the code.

Besides being shorter, it also shares the advantages of the functor version - being inliner friendly.

Share this post


Link to post
Share on other sites

This topic is 1991 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this