Segmentation fault during median function c-CodePudding

Does anyone know why I may be getting a segmentation fault during this stretch of code that returns the median of an already sorted vector v?

double median(vector<double> v){
    while(v.size() > 2){
        v.erase(v.begin());
        v.erase(v.end());
    }
    return (sum(v) / (v.size()));
}

CodePudding user response：

According to the C Standard the iterator used in the call of the member function erase of the class template std::vector shall be a valid dereferenceable const iterator.

However the iterator returned by the member function end is not a dereferenceable iterator because it does not point to a valid element of the vector.

It seems you mean

#include <vector>
#include <iterator>

//...

v.erase( std::prev( v.end() ) );

Also you need to check whether the passed vector is empty and its size is equal to 0.

Pay attention to that your approach is inefficient.

You could write for example

double median( const std::vector<double> &v )
{
    double value = 0.0;

    if ( not v.empty() )
    {
        value = v[v.size() / 2];
        if ( v.size() % 2 == 0 )
        {
            value = ( value   v[v.size() - 1] ) / 2;
        }
    }

    return value;
}

CodePudding user response：

Do you really have to remove elements from the vector to calculate the median? Since you are passing the input v by value, seems you don't. So why don't you just calculate the median on the values you actually need? Seems you have to calculate the median of the two middle values. So maybe something like this:

#include <vector>
#include <numeric>
#include <iostream>

using namespace std;

double median(vector<double> v){
   /* while(v.size() > 2){
        v.erase(v.begin());
        v.erase(v.end());
    }
    return (sum(v) / (v.size()));
    */
    vector<double> aux( v.begin()   v.size()/2 -1, v.begin()   v.size()/2  1);
    return ( accumulate(aux.begin(),aux.end(),0.0) / aux.size() );
    
}

int main()
{
    vector<double> myVec {0.1,0.2,0.3,0.4};
    cout<<median(myVec)<<endl;
    return 0;
}

You could change std::accumulate to your own sum function.