Expression: vector erase iterator outside range when using vector.erase()

Question

I am new to C++ and was trying to write a program that takes the lowest and the highest numbers in a vector, takes them away from the vector and proceeds to calculate an average with the rest of the numbers.

Now while I understand that I could do it after the numbers have been entered, I wanted to challenge myself and carry out these checks as the numbers were entered one by one. I was looking around but I couldn't find another person with the exact issue I am having (since I am using vector.end() - 1).

Below is my code and the error that it is giving me. Any guidance is much appreciated.

    #include <iostream>
    #include <iomanip>
    #include <vector>
    #include <algorithm>
    #include <numeric>
    #include <limits.h>
    
    using namespace std;
    typedef vector<int>::iterator iter;
    
    int main() {
    
        cout << "Please enter the series of scores: \n";
    
        // Read numbers from the standard input
        // and store them in a vector
        vector<int> scoreVector;
        int x = 0;
        int count = 0;
        // Assign the lowest possible value to the maximum possible integer so that any value entered initially will be lower that INT_MAX
        int lowestValue = INT_MAX;
        int highestValue = 0;
    
        iter lowestValueIndex;
        iter highestValueIndex;
    
        // Every time a number is entered, the program checks to see if it meets the criteria for the lowest or highest number entered so far
        while (cin >> x) {
    
            scoreVector.push_back(x);
    
            if (x > highestValue) {
                highestValueIndex = scoreVector.end() - 1;
                highestValue = x;
            }
            if (x < lowestValue) {
                lowestValueIndex = scoreVector.end() - 1;
                lowestValue = x;
            }
        }
    
        // Calculate the vector size
        auto n = scoreVector.size();
        
    
        if (n > 2) {
            
            cout << "\nThe lowest value is:" << lowestValue << "\nThe highest value is: " << highestValue << ". These values have been taken out." << '\n';
    
            // Remove the two values from the vector
            scoreVector.erase(lowestValueIndex);
            scoreVector.erase(highestValueIndex);
    
            // Calculate the average
            auto average = accumulate(scoreVector.begin(), scoreVector.end(), 0.0) / n;
    
            cout << "\naverage = " << average << '\n';
        }
        return 0;
    }

Answer 1

According to Iterator invalidation rules, scoreVector.push_back(x); may invalidate iterators taken before that by causing reallocation.

You should save indice instead of iterators as indice.

Also be careful because erase will shift following elements and it may affect indice of the element to be erased.

Try this:

    #include <iostream>
    #include <iomanip>
    #include <vector>
    #include <algorithm>
    #include <numeric>
    #include <limits.h>
    
    using namespace std;
    typedef vector<int>::iterator iter;
    
    int main() {
    
        cout << "Please enter the series of scores: \n";
    
        // Read numbers from the standard input
        // and store them in a vector
        vector<int> scoreVector;
        int x = 0;
        int count = 0;
        // Assign the lowest possible value to the maximum possible integer so that any value entered initially will be lower that INT_MAX
        int lowestValue = INT_MAX;
        int highestValue = 0;
    
        size_t lowestValueIndex = 0;
        size_t highestValueIndex = 0;
    
        // Every time a number is entered, the program checks to see if it meets the criteria for the lowest or highest number entered so far
        while (cin >> x) {
    
            scoreVector.push_back(x);
    
            if (x > highestValue) {
                highestValueIndex = scoreVector.size() - 1;
                highestValue = x;
            }
            if (x < lowestValue) {
                lowestValueIndex = scoreVector.size() - 1;
                lowestValue = x;
            }
        }
    
        // Calculate the vector size
        auto n = scoreVector.size();
        
    
        if (n > 2) {
            
            cout << "\nThe lowest value is:" << lowestValue << "\nThe highest value is: " << highestValue << ". These values have been taken out." << '\n';
    
            // Remove the two values from the vector
            scoreVector.erase(std::next(scoreVector.begin(), lowestValueIndex));
            if (lowestValueIndex < highestValueIndex) highestValueIndex--;
            scoreVector.erase(std::next(scoreVector.begin(), highestValueIndex));
    
            // Calculate the average
            auto average = accumulate(scoreVector.begin(), scoreVector.end(), 0.0) / n;
    
            cout << "\naverage = " << average << '\n';
        }
        return 0;
    }

Answer 2

Inside your while loop, you are taking iterators to scoreVector. However, every time you do a push_back on a vector, all iterators to it are invalidated. This means that when you exit your while loop, at least one if not both iterators are dangling. Passing those iterators to .erase then invokes undefined behavior.

Instead, you should separate out the reading of the variables, and computation of the iterators that you want to erase:

while (cin >> x) { 
    scoreVector.push_back(x);
}

iter lowestValueIndex = std::min_element(scoreVector.begin(), scoreVector.end());
scoreVector.erase(lowestValueIndex);

iter highestValueIndex = std::max_element(scoreVector.begin(), scoreVector.end());
scoreVector.erase(highestValueIndex);