@imren
Please use code tags (and, ideally, avoid a very non-portable conio.h).
By the time I had typed this, @repeater had given you the reason for your problem and @Thomas1965 had given you the best solution, but, with apologies to them, if you want to see some steps in the argument consider the following.
Your code fails because your for loop advances i each time, even if you moved an element forward using erase. Thus, if you have two negatives in succession (here, -5 and -2) then you will erase the -5, move the -2 into that i slot ... and then increment i without checking it. If you really want to remove one element at a time (and I
don't recommend it - I would go with @Thomas1965's solution) then you could do something like
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21
|
#include <iostream>
#include <vector>
using namespace std;
int main()
{
vector<int> B = { 3, -5, -2, 4, -7, 9, 22, -8 };
cout << "Before:\n";
for ( int x : B ) cout << x << endl;
int i = 0;
while ( i < B.size() )
{
if ( B[i] < 0 ) B.erase( B.begin() + i, B.begin() + i + 1 );
else i++; // only increase i if B[i] is NOT negative
}
cout << "After:\n";
for ( int x : B ) cout << x << endl;
}
|
A better solution is to use the
remove_if algorithm (needs
#include <algorithm>
) to move all negative elements to the end. It provides an iterator to the start of this, so we can then use erase to get rid of all negatives in one go:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21
|
#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;
bool isNegative( int i ) { return i < 0; }
int main()
{
vector<int> B = { 3, -5, -2, 4, -7, 9, 22, -8 };
cout << "Before:\n";
for ( int x : B ) cout << x << endl;
vector<int>::iterator it = remove_if( B.begin(), B.end(), isNegative );
B.erase( it, B.end() );
cout << "After:\n";
for ( int x : B ) cout << x << endl;
}
|
Actually, it is common to combine the
remove_if and
erase lines into one:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
|
#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;
bool isNegative( int i ) { return i < 0; }
int main()
{
vector<int> B = { 3, -5, -2, 4, -7, 9, 22, -8 };
cout << "Before:\n";
for ( int x : B ) cout << x << endl;
B.erase( remove_if( B.begin(), B.end(), isNegative ), B.end() );
cout << "After:\n";
for ( int x : B ) cout << x << endl;
}
|
Finally, the "predicate" function
isNegative() is only really used here, so we can put it directly into the erase/remove statement as what is called a
lambda function:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
|
#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;
int main()
{
vector<int> B = { 3, -5, -2, 4, -7, 9, 22, -8 };
cout << "Before:\n";
for ( int x : B ) cout << x << endl;
B.erase( remove_if( B.begin(), B.end(), []( int i ){ return i < 0; } ), B.end() );
cout << "After:\n";
for ( int x : B ) cout << x << endl;
}
|
Basically, we get @Thomas1965's solution here.
For the erase-remove idiom, see
https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom
Before:
3
-5
-2
4
-7
9
22
-8
After:
3
4
9
22 |
Yet another alternative is simply to push_back all the non-negative elements into a new, clean vector.