C++

Forum

 
Remove element from vector while iterating over it

ElXando (3)
Hi,

I'm trying to remove an element from a vector while iterating over it. Initially I was using a simple for i = 0, i < vector.size() but it wouldn't let me call remove(i) as i was an int, not an iterator.
So I've changed it to for(vector<Switch>::iterator i = switchVector.begin(); i != switchVector.end(); i++){ however when the condition to remove an element is met and switchVector.erase(i) is called I get an error: "Expression: vector iterator not dereferencable".
I've searched but can't seem to find a solution to this problem.
Thanks.
Catfish3 (666)
Please post your code.
ElXando (3)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
	for(vector<Switch>::iterator i = switchVector.begin(); i != switchVector.end(); i++){
		if(player1.getypos()-i->getypos() < i->getysize() && player1.getypos()-i->getypos() > -player1.getysize()){
		if(player1.getxpos()-i->getxpos() < i->getxsize() && player1.getxpos()-i->getxpos() > -player1.getxsize()){
			switchVector.erase(i);
			player2.setxpos(player1.getxpos()-25);
			player2.setypos(player1.getypos());
		}
		}
		if(player2.getypos()-i->getypos() < i->getysize() && player2.getypos()-i->getypos() > -player2.getysize()){
		if(player2.getxpos()-i->getxpos() < i->getxsize() && player2.getxpos()-i->getxpos() > -player2.getxsize()){
			switchVector.erase(i);
			player1.setxpos(player2.getxpos()-25);
			player1.setypos(player2.getypos());
		}
		}
	}
Fransje (435)
That is because you are trying to dereference switchVector.end(). That's an iterator beyond the last element of the vector, and therefore not dereferencable.

The reason why you reach .end() is that you also increment i if you erase something. That's wrong, because when you erase, all elements are shifted one place to the left, so i already points to the next element.
ElXando (3)
I see what you mean. Just tried adding i-- after the erase call, but now I get "Expression: vector iterator not decrementable".
vlad from moscow (6539)

@ElXando


If to speak shortly your code is so awful that the best advice is to throw out such a code. It does not deserve that somebody spend his time reviewing it.
Fransje (435)
"Expression: vector iterator not decrementable"

Not all iterators support decrementing. You can also try it like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
for ( vector < Switch >::iterator i = switchVector.begin(); i != switchVector.end(); i++ )
{
    if ( player1.getypos() - i->getypos() < i->getysize() && player1.getypos() - i->getypos() > -player1.getysize() )
    {
		if ( player1.getxpos() - i->getxpos() < i->getxsize() && player1.getxpos() - i->getxpos() > -player1.getxsize() )
        {
            if ( i == switchVector.end() )
            {
                break;
            }
            
			switchVector.erase(i);
			player2.setxpos(player1.getxpos()-25);
			player2.setypos(player1.getypos());
        }
    }
}
LB (13399)
.erase() returns a valid iterator to the next element in the vector. You have to do this:

MyIterator = MyVector.erase(MyIterator);

You will also need to not increment the iterator until after the next loop.


Example:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
#include <iostream>
#include <vector>

int main()
{
    typedef std::vector<int> MyVec_t;
    MyVec_t MyVec;

    for(MyVec_t::size_type i = 0; i < 10; ++i)
    {
        MyVec.push_back(i-1);
        MyVec.push_back(i+1);
        MyVec.push_back(i);
    }

    for(MyVec_t::const_iterator it = MyVec.begin(); it != MyVec.end(); ++it)
    {
        std::cout << *it << ' ' << std::flush;
    }

    std::cout << std::endl;

    for(MyVec_t::iterator it = MyVec.begin(); it != MyVec.end(); /*nothing*/)
    {
        if(*it == 7)
        {
            it = MyVec.erase(it);
        }
        else ++it;
    }

    for(MyVec_t::const_iterator it = MyVec.begin(); it != MyVec.end(); ++it)
    {
        std::cout << *it << ' ' << std::flush;
    }
}
http://ideone.com/oMVJWt
Last edited on
vlad from moscow (6539)
@L B
.erase() returns a valid iterator to the next element in the vector. You have to do this:

MyIterator = MyVector.erase(MyIterator);

You will also need to not increment the iterator until after the next loop.



There is also a problem that he is trying to remove the same iterator twice in the same loop.:)
Topic archived. No new replies allowed.