-1

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()));
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • 1
    [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Jesper Juhl Sep 11 '22 at 18:31
  • 2
    `end` returns an iterator to one element past the end, not a real `vector` element. There's nothing there to erase. Never tried to do it so I can't tell you for certain if this is the cause of the segfault. – user4581301 Sep 11 '22 at 18:31
  • 3
    Use `v[v.size() / 2]` ! –  Sep 11 '22 at 18:32

2 Answers2

2

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;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

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.

gudé
  • 89
  • 1
  • 12