vector iterator dereferencable?

[paul23]

Well first time I receive such a warning: I'm using VS 2008, and I get a debug-error (pop up window) when running my program:

Debug Assertion Failed!
Program: X\heaptest2.exe
File: Y\vc\include\vector
Line: 98

Expression: vector iterator not dereferencable
....

Well the code I use is simply:

class Student
{
private:
	template <typename T>
	class mycompare {
	public:
		bool operator() (const T& v1, const T& v2) const {
			return v1.first > v2.first;
		}
	};	
	template <typename T>
	class myfind : public std::binary_function<T, T, bool>
    {
	public:
		bool operator() (const T& v1, const T& v2) const {
			return v1.second > v2.second;
		}
	};
public:

// conventional constructor
	typedef std::pair<int,int> book;
	typedef heap_set<book,std::vector<book>,mycompare<book> > booklist ;

    Student()
	{	
    }
	void addbook(book b) {
		mybooklist.push(b);
	}
	booklist::iter find_if(int f,int l, int value) {
		std::vector<book> c(mybooklist._get_container());
		book t(std::make_pair(1,value));
		booklist::iter i(std::find_if(c.begin()+f,c.begin()+l,std::bind2nd(myfind<book>(),t)));
		std::cout << i->first << ' ' <<  i->second << std::endl;
		return i;
	}

	void print() {
		::PRINT_ELEMENTS(mybooklist._get_container());
	}
private:
	booklist mybooklist;
};



int main()
{
	Student s;
	for (int i=0;i<10;++i) {
		s.addbook(std::make_pair(::rand_range(1,20),::rand_range(1,20)*10));
	}
	s.print();
	*s.find_if(0,8,110);
	
    system("pause");
}

with "heap_set" being my personally-build priority queue (to have get_container function, as well as a few other things.)

the line "*s.find_if(0,8,110);" is what's causing the problem. Why can't I dereference the iteror I get from my custom find_if? - And what's the acceptable work-around? (other than creating a copy of the item and returning this copy).

[moorecm]

Does the heap_set's _get_container method return a reference to the underlying container?

I'm only speculating, as I am not familiar with heap_sets, but you could try changing line 32 to: std::vector<boo> & c(mybooklist._get_container());
My idea is to take a reference rather than making a local copy which is probably invalidating the iterator when the method returns--that is, if it is finding what you want. Do you get the expected output from cout on line 35?

Additionally, it seems odd not to check the return of std::find_if before accessing first and second on line 34 and 35. I don't think you can dereference an end(), for example.

While I'm at it, there are a number of other things about this snippet that seem out of the ordinary.

1. main does not have a return.

2. While not necessarily wrong, myfind and mycompare use > rather than <. I think the STL's convention was to use less than comparisons.

3. Just to mention a neat trick when using binary_fucntion, you could actually specify myfind::result_type rather than bool on line 15, and the same goes for the parameters--binary_function adds typedefs for the types so that you only have to specify them in one place.

4. In the snippet above, mycompare is never used.

5. Random access iterators are required for the find_if function because of the iterator arithmetic on line 34. Using advance would remove this additional requirement.

6. find_if, assuming you would like to remain consistant with the STL, should take iterators rather than integral offsets.

Hope this helps.

[paul23]

Well _get_container just does:

	    const container_type& _get_container() const {
		    return c;
	    }

c being the internal container - the problem shouldn't be here (aren't I returning as reference?) - Also changing the line 32 as suggested turn with an error: Cannot conver 'const std::vector<_Ty>' to 'std::vector<_Ty> &'...

2. this is just a test case to see where my general bigger code went wrong.. (I use > instead of == to be certain the values are found easier: and since it's sorted by).

4. - uhm it's needed for the heap_set?

6. again I made it use integers for easier debugging currently.. (This error along with a few others are driving me crazy)

thanks for the answers though :)

[moorecm]

There's your answer, I believe: _get_container does return a reference but it's a const reference.

To avoid making a local copy, you'll have to store the reference and the reference is being returned const, so try changing line 32 to:
const std::vector<book> & c(mybooklist._get_container());

Then you might have to change your iterators to const iterators, as well--I'm not sure, off the top of my head. Try the above and post any compiler errors. Do you intent to modify elements through the iterator or is this a read-only operation?

[jsmith]

Line 35 would certainly bomb if the requested element is not found and i is end(), as would line 55
since you dereferencing end() is undefined.

[paul23]

The std::cout function returns the expected values jsmith: so I know it's not the problem there (and as said this code is just to work out the problem: so I didn't put error checking on it yet).

I simply first removed all-constness from the vector returning as well as the iterator: this actually works!

However I would like the vector-returning to be a "constant" function (Changing the underlying vector would completely ruin the heapset, since heaps are specially sorted vectors): so I tested the line exactly above (and added constness to the _get_container function again.. Now some other compiler error happens:

h:\c++\heaptest2\heaptest2\heaptest.cpp(43) : error C2664: 'std::_Vector_iterator<_Ty,_Alloc>::_Vector_iterator(const std::_Vector_iterator<_Ty,_Alloc> &)' : cannot convert parameter 1 from 'std::_Vector_const_iterator<_Ty,_Alloc>' to 'const std::_Vector_iterator<_Ty,_Alloc> &'

(I've gotten this error a few times before, yet I still don't understand it...

EDIT: fixed it, by using a const_iterator instead of the normal iterator..

[jsmith]

Oh, yes, your find_if() function returns an iterator into the vector<> c, which is a local variable in the function.
So the iterator becomes invalid the moment the function returns since the local variable is destroyed.