how to remove the negative elements from the vector.

[imren]

#include <iostream>
#include<conio.h>
#include <vector>
using namespace std;
int main () {
vector<int>B(8);
cout<<"elements"<<endl;
B[0]=3;
B[1]=-5;
B[2]=-2;
B[3]=4;
B[4]=-7;
B[5]=9;
B[6]=22;
B[7]=-8;
for(int i=0;i<B.size();i++) {
cout<<B[i]<<" "<<endl;
}
for(int i=0;i<B.size();i++) {
if(B[i]<0)
B.erase(B.begin()+i,B.begin()+i+1);
}
for(int i=0;i<B.size();i++) {
cout<<B[i]<<" "<<endl;
}
getch ();
return 0;
}
what is the wrong my code?can anybody hepl me? pleae

[Repeater]

Vector:

3 -5 -2 4 -7 9 22 -8

In the loop to remove negative values, let's imagine we're got to the second value, "-5". So i=1.

We find -5, we erase it. The vector now looks like this:

3 -2 4 -7 9 22 -8

i=1 immediately after the erase, and then we increment i to go round the loop again.

So i=2. Which value are we going to examine now? The value "4". We skipped right over the "-2", because we shrunk the vector, making all the values slide to the left, and the "-2" went into the element we just looked at, and we never checkefd that element again.

[Thomas1965]

The recommended way is to use the remove_if and erase pattern.

#include <iostream>
#include <conio.h>
#include <vector>
#include <algorithm>

using namespace std;

int main() 
{
  vector<int> B = { 3, -5, -2, 4, -7, 9, 22, -8};
  
  cout << "Elements before removing the negative elements: " << endl;
  for (const int i : B)
    cout << i << " ";
  
  auto it = remove_if(B.begin(), B.end(),  [](const int i) {return i < 0; });

  B.erase(it, B.end());

  cout << "\nElements after removing the negative elements: \n" << endl;
  for (const int i : B)
    cout << i << " ";

  getch();
  return 0;
}

[lastchance]

@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

#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:

#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:

#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:

#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.

[keskiverto]

Note that std::vector has two versions of erase(). The one used in this thread takes two arguments and removes a range of elements. There is also a single-argument version that removes one element.

A logical, but very inefficient method is:

WHILE a negative element can be found
   erase that element

to use the remove_if algorithm (needs #include <algorithm> ) to move all negative elements to the end.

The remove_if does not "move removed elements to end". It overwrites removed elements if/when it moves kept elements towards the begin.