std::out_of_range When sorting through a vector

Viewed 46

I am trying to take an input number, make a vector with that many elements, take in those elements, and then sort the elements by largest to smallest. I believe I am close but I am getting an error :

Exited with return code -6 (SIGABRT).
terminate called after throwing an instance of 'std::out_of_range'
  what():  vector::_M_range_check: __n (which is 5) >= this->size() (which is 5)

Example: if the input is 5 10 4 39 12 2, the output should be 39,12,10,4,2,

Here is my code, I am thinking that I am getting the error from looping through my function, but I cannot figure out which one and I have tried changing all the loops.

#include <iostream>
#include <vector>
using namespace std;

void SortVector(vector<int>& myVec){
   int tempNum;
   for (int i = 0; i < myVec.size(); ++i){
      if (myVec.at(i) < myVec.at(i+1)){
         tempNum = myVec.at(i);
         myVec.at(i) = myVec.at(i + 1);
         myVec.at(i + 1) = tempNum;
      }
   }
}

void print(vector<int>& myVec){
   for(int i = 0; i < myVec.size(); ++i){
      cout << myVec.at(i)<<", ";
   }
}
      
int main() {
   int numOfVector;
   cin >> numOfVector;
   vector<int> myVec(numOfVector);
   
   for(int i = 0; i < numOfVector; ++i){
      cin >> myVec.at(i);
   }
   
   SortVector(myVec);
   print(myVec);
   return 0;
}
2 Answers

The loop of your sort runs through every element, and then examines the element that follows. When you are on the last index in the array, then there is no next element and so the at method throws std::out_of_range.

So you need to either loop up to size - 1, or start at index 1 and look at the previous. I prefer the latter because I don't like having size - 1 evaluated on every loop.

Something like this:

void SortVector(vector<int>& myVec)
{
   for (size_t i = 1; i < myVec.size(); ++i)
   {
      if (myVec[i - 1] < myVec.[i]) {
         std::swap(myVec[i - 1], myVec[i]);
      }
   }
}

Notice that I'm not using at. There is no point, because you have already range-checked the index by iterating over and indexing within a valid range.

I also used std::swap (from <utility>). Don't roll your own swaps. It's harder to read, and vulnerable to errors. Okay, maybe do it once in your life purely as an exercise and then use the proper method ever after.

It should be mentioned that your "sort" only does one pass of a bubble sort. You need an extra loop if you want to sort the entire vector.

You are accessing memory outside the vector when i is equal to myVec.size() - 1 in the for loop because in this case the expression myVec.at(i+1) is equivalent to myVec.at(myVec.size())

void SortVector(vector<int>& myVec){
   int tempNum;
   for (int i = 0; i < myVec.size(); ++i){
      if (myVec.at(i) < myVec.at(i+1)){
         tempNum = myVec.at(i);
         myVec.at(i) = myVec.at(i + 1);
         myVec.at(i + 1) = tempNum;
      }
   }

}

But in any case your function actually does not sort the vector because it is not enough to use only one for loop.

It seems you are trying to use the bubble sort method but it requires one more for loop.

And the code looked simpler if you used the standard function std::swap to swap adjacent elements of the vector. And there is no need to use the member function at instead of the subscript operator provided that the function is written correctly.

Instead of the manually written sorting function you could use the standard function std::sort as for example

#include <functional>
#include <iterator>
#include <algorithm>

void SortVector(vector<int>& myVec)
{
    std::sort( std::begin( myVec ), std::end( myVec ), std::greater<>() );
}

Here is a demonstration program.

#include <iostream>
#include <functional>
#include <vector>
#include <iterator>
#include <algorithm>

void SortVector( std::vector<int> &myVec )
{
    std::sort( std::begin( myVec ), std::end( myVec ), std::greater<>() );
}

int main( void ) 
{
    std::vector<int> myVec = { 5, 10, 4, 39, 12, 2 };

    for ( const auto &item : myVec )
    {
        std::cout << item << ' ';
    }
    std::cout << '\n';        

    SortVector( myVec );

    for ( const auto &item : myVec )
    {
        std::cout << item << ' ';
    }
    std::cout << '\n';        
}

The program output is

5 10 4 39 12 2 
39 12 10 5 4 2 
Related