Can someone please explain me why this code doesn't erase all 1s from the vector:
for (int i = 0; i < numbers.size(); i++)
{
if (numbers[i] == 1)
{
numbers.erase(numbers.begin() + i);
}
}
Can someone please explain me why this code doesn't erase all 1s from the vector:
for (int i = 0; i < numbers.size(); i++)
{
if (numbers[i] == 1)
{
numbers.erase(numbers.begin() + i);
}
}
Let's try it out.
Say you have a vector with the contents [1, 1, 1]
(all ones, just to keep it simple)
First iteration:
for (int i = 0; i < numbers.size(); i++) <-- i == 0; numbers.size() == 3
{
if (numbers[i] == 1) <-- true
{
numbers.erase(numbers.begin() + i); <-- erase is called on element #0
}
}
Second iteration:
The vector now contains [1, 1]
because we removed the 0th entry.
for (int i = 0; i < numbers.size(); i++) <-- i == 1; numbers.size() == 2
{
if (numbers[i] == 1) <-- true
{
numbers.erase(numbers.begin() + i); <-- erase is called on element #1
}
}
Third iteration:
The vector now contains [1]
for (int i = 0; i < numbers.size(); i++) <-- i == 2; numbers.size() == 1; the loop condition is false, so we exit the loop
{
if (numbers[i] == 1)
{
numbers.erase(numbers.begin() + i);
}
}
Final result:
the vector contains [1]
.
As you can probably see from evaluating the code line by line manually, the problem is that you increment i
even after removing an element. Every time you remove an element, you shift all the remaining elment to lower indices, but at the same time, you increment the counter so the next index you look at is one higher. So when you remove element i
, the element that was previously index i+1
is moved to index i
. But in the next iteration, you no longer look at index i
, but instead i+1
, so you skip past an element without ever looking at it.
Your code says i < numbers.size(), and you increment i and also delete n[i](if it has 1) After deleting , you should not increment i because a new number gets in the currrent position of i.
If numbers[]={0,1,2,3,1,4} -> you get numbers[]={0,2,3,4}
If numbers[]={0,1,1,2,3,1,4} -> you get numbers[]={0,1,2,3,4},
i.e after deleting , it skips the next number just add i--; after deleting numbers[i]
for (int i = 0; i < numbers.size(); i++){
if (numbers[i] == 1){
numbers.erase(numbers.begin() + i);
i--;
}
}
This will give you the correct answer.
When an element of a vector is erased then all elements after the erased element are moved left. So for example if the element with index 0 was erased then the first actual element of the vector will have index equal to 9. You must not to increase the index in this case.
The valid loop can look like
for ( int i = 0; i < numbers.size(); )
{
if ( numbers[i] == 1 )
{
numbers.erase( numbers.begin() + i );
}
else
{
++i;
}
}
Take into account that it is simpler to apply standard algorithm std::remove
declared in header <algorithm>
For example
#include <algorithm>
//...
numbers.erase( std::remove( numbers.begin(), numbers.end(), 1 ), numbers.end() );
Here is a demonstrative example
#include <iostream>
#include <vector>
#include <algorithm>
int main()
{
std::vector<int> v = { 1, 1, 2, 1, 3, 1, 4, 1, 5 };
for ( int x : v ) std::cout << x << ' ';
std::cout << std::endl;
v.erase( std::remove( v.begin(), v.end(), 1 ), v.end() );
for ( int x : v ) std::cout << x << ' ';
std::cout << std::endl;
return 0;
}
The output is
1 1 2 1 3 1 4 1 5
2 3 4 5