CODEWITHSUNDEEP
c++stl
venkysmarty
venkysmarty

Reputation: 11441

crash when using upper_bound in C++

I have following program which crashed at upper bound call. I am not getting why there is a crash. Any reason why I am having a crash. Thanks for your help and time.

#include <iostream>
#include <algorithm>
#include <vector>

using namespace std;

enum quality { good = 0, bad, uncertain };

struct sValue {
   int time;
   int value;
   int qual;
};

struct CompareLowerBoundValueAndTime { 
    bool operator()( const sValue& v, int time ) const {
        return v.time < time;
    } 

    bool operator()( const sValue& v1, const sValue& v2 ) const { 
        return v1.time < v2.time; 
    }

    bool operator()( int time1, int time2 ) const { 
        return time1 < time2; 
    }   

    bool operator()( int time, const sValue& v ) const  { 
        return time < v.time;    
    } 
}; 

struct CompareUpperBoundValueAndTime { 
    bool operator()( const sValue& v, int time ) const {
        return v.time > time;
    } 

    bool operator()( const sValue& v1, const sValue& v2 ) const { 
        return v1.time > v2.time; 
    }

    bool operator()( int time1, int time2 ) const { 
        return time1 > time2; 
    }   

    bool operator()( int time, const sValue& v ) const  { 
        return time > v.time;    
    } 
}; 

class MyClass {

public:
    MyClass() {
        InsertValues();
    }

       void InsertValues();

       int GetLocationForTime(int time);

       void PrintValueContainer();

private:

    vector<sValue> valueContainer;
};

void MyClass::InsertValues() {
    for(int num = 0; num < 5; num++) {
        sValue temp;
        temp.time = num;
        temp.value = num+1;
        temp.qual = num % 2;
        valueContainer.push_back(temp);
    }
}

void MyClass::PrintValueContainer()
{
    for(int i = 0; i < valueContainer.size(); i++) {
        std::cout << i << ". " << valueContainer[i].time << std::endl;
    }
}




int MyClass::GetLocationForTime(int time)
{
    std::vector< sValue >::iterator lower, upper;
    lower =  std::lower_bound(valueContainer.begin(), valueContainer.end(), time, CompareLowerBoundValueAndTime() );

    upper =  std::upper_bound(valueContainer.begin(), valueContainer.end(), time, CompareUpperBoundValueAndTime() ); // Crashing here.

    std::cout << "Lower bound: " << lower - valueContainer.begin() << std::endl;
    std::cout << "Upper bound: " << upper - valueContainer.begin() << std::endl;

   return lower - valueContainer.begin();
}



int main()
{
    MyClass a;
    a.PrintValueContainer();
    std::cout << "Location received for 2: "  << a.GetLocationForTime(2) << std::endl;
    return 0;
}

Upvotes: 1

Views: 1308

Answers (4)

Alex
Alex

Reputation: 7858

lower_bound and upper_bound work on a sorted sequence. The sequence has to be sorted using the same comparing function that you pass to both functions.

When you insert the elements in InsertValues you insert them in ascending order, so your CompareLowerBoundValueAndTime is a correct way to compare them.

But for upper_bound you're passing a different compare function. Pass CompareLowerBoundValueAndTime() and it should work.

Note that CompareLowerBoundValueAndTime is a misleading name. It should be something along the lines of CompareValueAndTimeAscending.

Upvotes: 8

CashCow
CashCow

Reputation: 31445

I think you are getting an assertion error in upper_bound because it is finding that your sequence is not correctly sorted.

You seem to misunderstand what upper_bound does. It is the same as lower_bound except that the item pointed to by the iterator will be strictly greater than the search value, not greater-or-equal. IF there are no such values, it will point to the end of the sequence.

When using a predicate (Pred), it needs to be sorted such that

 Pred( iter2, iter1 )

will return false whenever iter2 appears later than iter1 in the sequence.

That is not the case with your sequence and predicate combination, therefore you are getting an assertion error.

Upvotes: 0

alestanis
alestanis

Reputation: 21863

Your compiler is giving you the answer. Check your code here: http://ideone.com/x6RE9

This gives you an error saying:

prog.cpp: In member function ‘int MyClass::GetLocationForTime(int)’:
prog.cpp:94: error: no match for ‘operator*’ in ‘*upper.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator* [with _Iterator = sValue*, _Container = std::vector<sValue, std::allocator<sValue> >]()’

You don't have to dereference upper twice, it doesn't make any sense.

Upvotes: 1

Bo Persson
Bo Persson

Reputation: 92381

You should use the same comparer for both upper_bound and lower_bound. The difference is in the algorithm, not in the comparison.

Upvotes: 1

Related Questions