functor won't change internal data

[strawberry]

When called in adjacent_find, the p pred variable's data member decide does not change. Why? I even made it public, though it should be private, not that it matters since operator() should be able to modify it anyway.

#include<vector>
#include<iterator>
//#include<>

using namespace std;


class pred{ //deciding functor
public:
	int decide;
	pred(int dec_):decide(dec_){}
	bool operator()(int lhs, int rhs){
		int diff(rhs-lhs);
		if(decide == 0){
			switch(diff){
				case(1): 
					decide = 1; 
					return true;
				case(-1): 
					decide =-1;
					return true;
			}
			decide = 0;
			return false;
		}
		if((rhs-lhs) == decide)
			return false;
		else{
			decide = 0;
			return true;
		}
	}
};

//--------------------------------------------------------------------------------------------
int main()
{
	int seq[]={1,2,3,5,6,7,8,6,5,4,3,4,4,3,2,1,0};
	vector<int> veci(seq, seq+sizeof(seq)/sizeof(int));
	
	typedef vector<int>::iterator VII;
	VII first = veci.begin();
	VII last = veci.end();
	pred p(0);
	int longest(0); 
	while( first != last ){
		first = adjacent_find(first, last, p);
		VII lastInSeq = adjacent_find(first, last, p);
		int len = distance(first, lastInSeq)+1;
		if(len > longest) longest = len;
		first = ++lastInSeq;
	}
    return 0;
}

[Robertlzw]

The program compiles, but has the following run-time exception on VC++2008:
_Myt& operator++()
{ // preincrement
_SCL_SECURE_VALIDATE(this->_Has_container());
_SCL_SECURE_VALIDATE_RANGE(_Myptr < ((_Myvec *)(this->_Getmycont()))->_Mylast); // this line

[strawberry]

I simplified the code so it is easier to see what is happening. operator() should change the value of decide of p in adjacent_find, but it doesn't. It seems like a temporary copy of p is made in adjacent_find. Is it so? If so, how not to make this temporary copy but use p itself in adjacent_find?

class pred{ //deciding functor
	int decide;
public:
	pred(int dec_):decide(dec_){}
	bool operator()(int lhs, int rhs){
		decide = 1;
		return true;
	}
	void print() {cout << decide << endl;}
};


//--------------------------------------------------------------------------------------------
int main()
{
	int seq[]={1,2,3};
	vector<int> veci(seq, seq+sizeof(seq)/sizeof(int));
	pred p(0);
	adjacent_find(veci.begin(), veci.end(), p);
	p.print();
    return 0;
}

[Duthomhas]

It is working fine. Your test fails.

The adjacent_find() algorithm does not take a reference to your functor -- it takes a copy. So any changes that occur are in the copy, not in the original instance (p).

Also, your functor always returns true, so the adjacent_find() algorithm terminates immediately (since the first two elements in the list are considered to be adjacent).

Try this to see what is happening:

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

//--------------------------------------------------------------------------------------------

class pred{ //deciding functor
	int decide;
public:
	pred(int dec_): decide(dec_)
	{
		cout << "(ctor " << decide << ")\n";
	}
	pred(const pred& p): decide(p.decide)
	{
		cout << "(copy-ctor " << decide << ")\n";
	}
	pred& operator = ( const pred& p )
	{
		decide = p.decide;
		cout << "(op= " << decide << ")\n";
		return *this;
	}
	bool operator()(int lhs, int rhs){
		cout << "decide = " << decide << endl;
		decide += 1;
		return false;
	}
};


//--------------------------------------------------------------------------------------------
int main()
{
	int seq[]={1,2,3,4,5};  // N-elt array produces N-1 comparisons
	vector<int> veci(seq, seq+sizeof(seq)/sizeof(int));
	pred p(0);
	adjacent_find(veci.begin(), veci.end(), p);
	return 0;
}

Hope this helps.

[player6]

Hi Duoas, I have one problem:
the function adjacent_find is a template inline function, why it produce a temproy copy rather than expand the code just like a macro?
I test it from the vc 2005.

template<class _FwdIt,
	class _Pr> inline
	_FwdIt adjacent_find(_FwdIt _First, _FwdIt _Last, _Pr _Pred)
	{	// find first satisfying _Pred with successor
	_ASSIGN_FROM_BASE(_First,
		_Adjacent_find(_CHECKED_BASE(_First), _CHECKED_BASE(_Last), _Pred));
	return (_First);
	}

[strawberry]

Thanks for the clarification, Duoas. It's good to learn that there is a copy of the predicator in algorithms. Apparently, I need to use a static data member to keep the value of decide in scope. It does not look very elegant, though. - Anyway, what I am trying to do is find the length of the longest increasing or decreasing sequence (step 1 or -1) in the container. There should be a much nicer solution to this problem using STL algorithms, but I am just not finding it. If someone knows a nicer one, let me know.

#include<vector>
#include<iterator>
#include<iostream>

using namespace std;


class pred{ //deciding functor
	static int decide;
public:
	bool operator()(int lhs, int rhs){
		int diff(rhs-lhs);
		if(decide == 0){
			switch(diff){
				case(1): 
					decide = 1; 
					return true;
				case(-1): 
					decide =-1;
					return true;
			}
			decide = 0;
			return false;
		}
		if((rhs-lhs) == decide)
			return false;
		else{
			decide = 0;
			return true;
		}
	}
};

//--------------------------------------------------------------------------------------------
int pred::decide = 0;

int main()
{
	int seq[]={1,2,3,5,6,7,8,6,5,4,3,4,4,3,2,1,0,1};
	vector<int> veci(seq, seq+sizeof(seq)/sizeof(int));
	
	typedef vector<int>::iterator VII;
	VII first = veci.begin();
	VII last = veci.end();
	pred p;
	int longest(0); 
	while( first != last ){
		first = adjacent_find(first, last, p);
		VII lastInSeq = adjacent_find(first, last, p);
		int len = distance(first, lastInSeq)+1;
		if(len > longest) longest = len;
		first = lastInSeq;
	}
    return 0;
}

[jsmith]

Rather than making it static, you should make it a reference, then write a constructor
that takes an int reference as parameter.

class pred {
    int& decide;
  public:
    pred( int& d ) : decide( d ) {}
};