I am trying to iterate through a vector and erase specific items from it. I'm working form the end of the vector down so that I don't mess up the iterator as items are removed, but when I try to compile it's throwing an error. I looked through some of the other posts with this same error but didn't see anything that applied to my situation, or if there was I didn't catch it, I'm still pretty new to C++ and programming in general. Below is a sample of a simpler code to illustrate my problem.
#include <iostream>
#include <vector>
using namespace std;
int vectorErase(vector<int>, int &);
int main()
{
vector<int> test;
for(int i=0; i<11;i++)
{
test.push_back(i);
cout<<test[i];
}
for(int i=10;i<=0;i--)
{
vectorErase(test, i);
cout<<test[i];
}
system("pause");
return 0;
}
int vectorErase(vector<int> test, int &iterat)
{
if(test[iterat]>6)
{
test.erase(iterat);
}
return 0;
}
any help would be great
The most immediate problems in your code are:
- Passing your vector by value, so the original is never modified.
- Not using
erase()
properly. It returns an iterator to the next element in the sequence, which you did not erase (yet). This means if you use iterators and remove an element, you need not (and should not) increment the iterator. An example is forth-coming.
- In conjunction with the above, simply, you're not using iterators, and you should be.
Your code could do without the function and simply do this:
#include <iostream>
#include <vector>
using namespace std;
int main()
{
vector<int> test;
for(int i=0; i<11;i++)
{
cout << i << ' ';
test.push_back(i);
}
cout << '\n';
for(auto it = test.begin(); it != test.end();)
{
if (*it > 6)
it = test.erase(it);
else
{
cout << *it << ' ';
++it;
}
}
cout << '\n';
return 0;
}
Output
0 1 2 3 4 5 6 7 8 9 10
0 1 2 3 4 5 6
I strongly advise you spend a few days working with iterators. Start with something simple (like this example).
I am trying to iterate through a vector and erase specific items from it. I'm working form the end of the vector down so that I don't mess up the iterator as items are removed,
In addition to passing by reference instead of by value, instead of writing loops and worry about invalidating iterators, learn to use algorithms, more specifically the erase/remove_if
idiom for containers such as vector. It is very easy even for good C++ programmers to make a mistake, which is why the algorithms should be used.
Here is a sample of using the idiom (erase/remove_if).
#include <algorithm>
//...
bool IsGreater(int val) { return val > 6; }
//...
test.erase(std::remove_if(test.begin(), test.end(), IsGreater), test.end());
The remove_if takes the items that satisfy the condition and moves them to the end of the vector. The return value of remove_if() is the iterator that points to the beginning of the moved items. Then the erase() takes the items and deletes them from the vector.
The advantage of doing things this way are many, but one of them is that you no longer need to worry about "messing up iterators". It is very hard to mess this up -- the one way to mess up is either you provide wrong iterator types (then you will get a syntax error), or your comparison function doesn't work (easily fixed). But at runtime, there is little to no chance of using invalid iterators.
The other advantage is that any good C++ programmer can immediately understand what the erase/remove_if() does. If I look at your code and you never told us what it did, I would
1) Have to read it a few times to get what's going on and
2) Have to run it under a debugger to see if it does what I think it does, and do it correctly.
With the algorithms, I know right away what the code does, and even more important, the code works without having to run it under a debugger.
Note that the sample I provided uses a simple function IsGreater(). The other ways to write the test function is to use std::greater<> (along with a std::bind1st), using a function object, using a lambda, etc. But I provided probably the easiest way to initially understand what is happening.
You're passing a copy of your vector to vectorErase
, so any changes it makes to its copy have no effect on the original.
If you want to modify the vector passed to the function you need to take a reference to the original, rather than a copy. This is easy - instead of vector<int> test
, write vector<int> & test
.
(You are passing iterat
by reference, which you don't need. Did you just put the &
in the wrong place?)
There are a few problems. First, there's the fact that you must pass the vector by reference to affect the original.
int vectorErase(vector<int>&, int );
Then there's the problem of the second loop:
for(int i=10;i>=0;i--)
{
vectorErase(test, i);
cout<<test[i] << ' ';
}
And finally the function itself with the proper use of erase
:
int vectorErase(vector<int> &test, int iterat)
{
if(test[iterat]>6)
{
test.erase(test.begin()+iterat);
}
return 0;
}