Appending a std::vector with its own elements using iterators

Question

The following code works as expected (the test passes) but I wonder if working with iterators in this way is considered a bad practice in c++ or if it is okay.

Maybe this is specific for std::vector and other collections behave differently and best practices vary between collections (or even their implementations)?

It certainly is not okay in other languages and most of the time changing a collection will invalidate iterators and throw exceptions.

BOOST_AUTO_TEST_CASE (ReverseIteratorExample) {
    std::vector<int> myvector;
    for(int i = 0; i < 5; i++)
    {
        myvector.push_back(i);
    }

    // is this generally a bad idea to change the vector while iterating?
    // is it okay in this specific case?
    myvector.reserve(myvector.size() + myvector.size() - 2 );
    myvector.insert(myvector.end(), myvector.rbegin() + 1, myvector.rend() -1);

    int resultset [8] = { 0,1,2,3,4,3,2,1 };
    std::vector<int> resultVector( resultset, resultset + sizeof(resultset)/sizeof(resultset[0]) );
    BOOST_CHECK_EQUAL_COLLECTIONS(myvector.begin(), myvector.end(), resultVector.begin(), resultVector.end());
}

Summarized Questions:

1. Is this generally a bad idea to change the vector while iterating?
2. Is it okay in this specific case?
3. Is this specific for std::vector and other collections behave differently?
4. Do best practices vary between collections (or even their implementations)?

Answer 1

This is not valid code. The standard's definition of operations on sequence containers states (23.2.3@4):

a.insert(p,i,j) - [...] pre: i and j are not iterators into a.

So your code invokes undefined behavior because it violates the precondition for the insert operation.

If instead of using insert, you wrote a loop iterating from myvector.rbegin() + 1 to myvector.rend() -1 and called push_back on all values, your code would be valid: This is because push_back only invalidates vector iterators if a reallocation is needed, and your call to reserve ensures that this is not the case.

In general, while there are some cases where modifying a container while iterating over it is fine (such as the loop described above), you have to make sure that your iterators aren't invalidated while doing so. When this happens is specific to each container.