for loop doesn't end when condition is met

Question

for(it = gameObjects.begin();it!=gameObjects.end();it++){
    it->second->update(frameTime);
    if(it->second->getSprite()->GetPosition().y > 500){
        std::cout << "Removing enemy" << std::endl;
        std::map<sf::String,VisibleGameObject*>::iterator itor = Remove(it->second->getName());
        if(itor!=gameObjects.end()){
            std::cout << "itor doesn't equal" << std::endl;
            it=itor;
        }else{
            std::cout << "itor = end" << std::endl;
            it=itor;
        }
    }
}

As soon as itor = end is printed, it errors - "map set iterator not incrementable". I thought the for loop should end before it increments again, as it!=gameObjects.end() will be false after this. Adding a break in the else statement resolves the problem.

Why doesn't it work without the break? I'm assuming it's something to do with when the iterator is incremented compared to when the condition is checked.

obmarg · Accepted Answer · 2012-01-28 16:12:48Z

2

You assume correct. The iterator is incremented at the end of the loop, and then the condition is checked.

So after "itor = end" is printed, it gets incremented to gameObjects.end()++ which of course is not valid. You could get around by checking for itor == gameObjects.end() inside the loop, and then breaking.

EDIT: As pointed out in the comments, you're better just removing the ++it from the loop, to avoid skipping over the element after a removed element. For example:

for( it = gameObjects.begin(); it!=gameObjects.end(); ) {
    ...
    if(it->second->getSprite()->GetPosition().y > 500) {
        it = Remove( it->second->getName() );
    } else {
        ++it;
    }
}

edited Jan 28, 2012 at 16:12

answered Jan 28, 2012 at 14:53

obmarg

9,60539 silver badges60 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Bo Persson Over a year ago

There's still a problem here. After it=itor; and it++ you just skipped one element. If there is ever a need to remove two in a row, the code will miss the second one.

Bo Persson Over a year ago

Sorry, but it is still a bit tricky. If you delete the very first element, you cannot decrement the iterator (because it will be == begin()). The standard is to remove it++ from the for and then either remove an element or increment the iterator.

obmarg Over a year ago

@BoPersson Ah yeah, thanks for pointing that out. Clearly wasn't paying as much attention to my example as I was to the initial problem. Fixed now (I hope)

Alican · Accepted Answer · 2012-01-28 14:54:47Z

2

for loop first performs the statement (increments the value) then checks the condition. the problem is that for-loop tries to increment the iterator after it has reached the end of the map.

answered Jan 28, 2012 at 14:54

Alican

2861 silver badge8 bronze badges

Comments

N_A · Accepted Answer · 2012-01-28 14:55:05Z

1

std::map<sf::String,VisibleGameObject*>::iterator itor = Remove(it->second->getName());
.
.
.
it=itor;

is your problem.

At some point in time the remove statement removes the last item from gameObjects and then you do it=itor, setting it to the last item in gameObjects without checking your looping condition. for loops increment at the end of the loop for ++ so then you're past the end of gameObjects.

answered Jan 28, 2012 at 14:55

N_A

20k4 gold badges55 silver badges98 bronze badges

Comments

Rune FS · Accepted Answer · 2012-01-28 15:14:05Z

1

Assuming that you can do with out the console output you could simplify the loop a bit and simply do as below:

for(it = gameObjects.begin();it!=gameObjects.end();){
    it->second->update(frameTime);
    it = it->second->getSprite()->GetPosition().y > 500 
         ? Remove(it->second->getName()); 
         : it++;
}

answered Jan 28, 2012 at 15:14

Rune FS

21.7k7 gold badges65 silver badges101 bronze badges

2 Comments

pighead10 Over a year ago

thanks, I didn't know the syntax for that in C++ but I did try something along those lines before reverting back

Rune FS Over a year ago

@PigHead The last statement of the body could also be places in the loop header making the body a one-liner but I personally find this easier to read than have such a long statement in the loop header

Collectives™ on Stack Overflow

for loop doesn't end when condition is met

4 Answers 4

3 Comments

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related