2

I want to run a loop on a std::vector<std::vector<int>> array to delete desired elements.

I'm using for (i = 0; i < array.size(); i++)

is it safe to invoke array.erase() from within the loop?

In my head I assume for checks array.size() at every iteration, but maybe it only does it once at initiation. If it did, for (i = 0; i < &array.size(); i++) would work a fine solution, would it not?

Thank you.

aName
  • 245
  • 1
  • 6
  • array is actually a vector of vectors (`std::vector> array`). But reason I think &array.size() might work is because if `for` grabs the result of array.size() once at the beginning only, then I'm guessing it does so by pass by copy. So to get the address-of will solve that – aName Apr 24 '19 at 06:12
  • Please use the remove/erase idiom: https://stackoverflow.com/a/347478/283561 – Simon Apr 24 '19 at 06:13
  • @Simon, I don't understand why erasing an element in a vector is so complex, but anyway, I thought this bit of simplified code might work? `array.erase(array.begin() + i);` – aName Apr 24 '19 at 06:19

4 Answers4

5

is it safe to invoke array.erase() from within the loop?

Yes, it is perfectly safe. Compiler is allowed to optimize the check by calling array.size() only once out of the loop only if it can prove that vector is not modified inside the loop, e,g, if the vector is constant and thus not modifiable. In that case optimizing that way won't change observable behavior. But since you are calling erase inside the loop compiler is not allowed to call array.size() only once.

In general, an implementation is allowed to do only those code transformations that do not change observable behavior. This is called as-if rule.

taskinoor
  • 43,612
  • 11
  • 111
  • 136
4

I'm using for (i = 0; i < array.size(); i++)

is it safe to invoke array.erase() from within the loop?

It is safe but it is going throw your iteration logic off.

Let's say you have {1 2 3 4 5} in array.

You removed 2 from the array. At that time, i is 1. After removing 2, array is now {1 3 4 5}. You increment i in the for statement, which makes i 2. You will end up accessing 4 from the array in the loop, which skips 3 altogether.

In my head I assume for checks array.size() at every iteration, but maybe it only does it once at initiation.

No, it does that check in every iteration of the loop.

If it did, for (i = 0; i < &array.size(); i++) would work a fine solution, would it not?

Not sure where you got that idea but it's wrong. Don't even go there.


You can use:

for ( i = 0; i < array.size(); /* Don't increment here */ )
{
    // Code

    // Check for erase.
    if ( check-for-erase-logic )
    {
        // Erase item
        // Don't increment iteration counter.
    }
    else
    {
       // Increment iteration counter.
       ++i;
    }
}
Community
  • 1
  • 1
R Sahu
  • 196,807
  • 13
  • 136
  • 247
2

i < array.size() is the condition part of the for statement which is evaluated before each iteration, and if it yields false, the loop is exited. This answers the question in your title.

However, erase will invalidate iterators. So you should be careful. It is better to use algorithms to achieve this. One way would be the remove/erase idiom.

P.W
  • 24,743
  • 6
  • 32
  • 69
1

In contrast to a range for loop aka for (T element : container), in normal loops the condition is checked every time. So yes your normal for loop is safe.

However be aware of iterator, references and pointer invalidation after erase, and also that your i could be outside of the new range after the erase

Here some do's and dont's

#include <iostream>
#include <vector>

int main() {
    using T = std::vector<int>;
    std::vector<T> vector{{3,4},{3,5},{5,9}};

    for (size_t i = 0; i < vector.size(); ++i) {
        auto &vec2 = vector.at(2);
        if (i == 2) {
            vector.erase(vector.begin() + 1); //fine}
        }
        auto &v = vector[i]; //NOT FINE UNDEFINED BEHAVIOUR!
        if (i < vector.size()) //Could be optimized out by compiler because of UB!, when compiling with line above,
            // as the for check
            break;
        std::cout << vec2.at(1); //Also UNdefined behaviour since many things are invalidated after erase
    }
    for (auto v : vector) {
        vector.erase(vector.begin()); //Also NOT ALLOWED since for ranges are allowed to cache iterators
    }
}
Superlokkus
  • 3,784
  • 1
  • 17
  • 49