Sorting A vector of objects

[FITorion]

I have a vector full of Student Objects.
Student has:
unsigned int idnumber;
std::string name;
std::vector<int> grades;

I've overloaded < to sort the Students based of their average grade.
Here that is:

inline bool operator< (Student &a, Student &b){
	if(a.mean() < b.mean()){
		return true;
	}
	return false;
}

I've defined a function to sort the students based off their name.
Here that is:

inline bool Student::compAlph(Student &a, Student &b)const{
	return a.getName().compare(b.getName())==0;
}

I'm trying to then use the <algorithm> sort() on the vector and getting an error that I haven't been able to figure out.
Here are my attempts:

std::sort<std::vector<Student>::iterator, Student>(list.begin(), list.end(), list[0]);
std::sort<std::vector<Student>::iterator, bool>(list.begin(), list.end(), &Student::compAlph);

and Here is the error:
algorithm(3191) : error C2064: term does not evaluate to a function taking 2 arguments

[Albatross]

Apparently you have a term that is not a function that takes two arguments but is being treated like one. We... don't see that in your code, so we can't really help you with finding it.

-Albatross

[FITorion]

As you can see by my line:

algorithm(3191) : error C2064: term does not evaluate to a function taking 2 arguments

The error is in <algorithm> Which is inherent to C++ and not something I wrote. Now obviously the error can't be with <algorithm> so something I'm providing it must be causing this error... I've provided the only part of my code that uses <algorithm>.

But if you really want to see the code that C++ comes with that I'm not going to be able to change... here it is:

template<class _RanIt,
	class _Pr> inline
	pair<_RanIt, _RanIt> _Unguarded_partition(_RanIt _First, _RanIt _Last,
		_Pr _Pred)
	{	// partition [_First, _Last), using _Pred
	_RanIt _Mid = _First + (_Last - _First) / 2;
	std::_Median(_First, _Mid, _Last - 1, _Pred);
	_RanIt _Pfirst = _Mid;
	_RanIt _Plast = _Pfirst + 1;

	while (_First < _Pfirst
		&& !_DEBUG_LT_PRED(_Pred, *(_Pfirst - 1), *_Pfirst)
		&& !_Pred(*_Pfirst, *(_Pfirst - 1)))                                //this is line 3191
		--_Pfirst;
	while (_Plast < _Last
		&& !_DEBUG_LT_PRED(_Pred, *_Plast, *_Pfirst)
		&& !_Pred(*_Pfirst, *_Plast))
		++_Plast;

	_RanIt _Gfirst = _Plast;
	_RanIt _Glast = _Pfirst;

	for (; ; )
		{	// partition
		for (; _Gfirst < _Last; ++_Gfirst)
			if (_DEBUG_LT_PRED(_Pred, *_Pfirst, *_Gfirst))
				;
			else if (_Pred(*_Gfirst, *_Pfirst))
				break;
			else
				std::iter_swap(_Plast++, _Gfirst);
		for (; _First < _Glast; --_Glast)
			if (_DEBUG_LT_PRED(_Pred, *(_Glast - 1), *_Pfirst))
				;
			else if (_Pred(*_Pfirst, *(_Glast - 1)))
				break;
			else
				std::iter_swap(--_Pfirst, _Glast - 1);
		if (_Glast == _First && _Gfirst == _Last)
			return (pair<_RanIt, _RanIt>(_Pfirst, _Plast));

		if (_Glast == _First)
			{	// no room at bottom, rotate pivot upward
			if (_Plast != _Gfirst)
				std::iter_swap(_Pfirst, _Plast);
			++_Plast;
			std::iter_swap(_Pfirst++, _Gfirst++);
			}
		else if (_Gfirst == _Last)
			{	// no room at top, rotate pivot downward
			if (--_Glast != --_Pfirst)
				std::iter_swap(_Glast, _Pfirst);
			std::iter_swap(_Pfirst, --_Plast);
			}
		else
			std::iter_swap(_Gfirst++, --_Glast);
		}
	}

[Albatross]

...wait... is _Pred a macro? The preceding underscore makes me think it is, as there is a conversion to place an underscore in the names of macros, before anything else. If so, I wonder what's in it.

-Albatross

[FITorion]

I don't have any idea...

If you want to debug a standard template library I suggest you
#include <algorithm>

and step into it. I don't think the 8192 character limit would be enough for me to post the whole thing.

Again I have no control over this code... so something in the code I do have control over must be causing this...

[Albatross]

Wait... /desk

That's the C++ standard library. At 8:30 my awareness plummets. Lemme get a cup of coffee... okay, now that I have it, let's try this again.

So. Read this, and I think that will solve your problem.
http://cplusplus.com/reference/algorithm/sort/

Especially, read about comp. Apparently, your function does not take two arguments.

Looking through the example may help you to understand what is needed.

-Albatross

[FITorion]

Yeah I've read that... I've been rereading it over and over for about the past week.

how does:

inline bool operator< (Student &a, Student &b){
	if(a.mean() < b.mean()){
		return true;
	}
	return false;
}

and or

inline bool Student::compAlph(Student &a, Student &b)const{
	return a.getName().compare(b.getName())==0;
}

not take 2 arguments?

[Albatross]

They both take two arguments, however somehow I suspect that the reference might be messing things up.
&Student::compAlph)

-Ablatross

[FITorion]

when I remove that:

std::sort<std::vector<Student>::iterator, bool>(list.begin(), list.end(), Student::compAlph);

I get this error:

gradebook.cpp(124) : error C3867: 'Student::compAlph': function call missing argument list; use '&Student::compAlph' to create a pointer to member

[Albatross]

Okay... could you define a comparison function outside of the class but declare it inside the class as a friend?

-Albatross

[FITorion]

um... what?

How would I do that?

add friend to it in the Student.h? and remove Student:: from it where it's defined in gradebook.cpp?

[Albatross]

That sounds about right, however to be sure, declare it again outside of the class's declaration in Student.h.

Oh, and remove Student:: from all other instances of it. But... you knew that.

EDIT: It's getting quite late here and I had a poor night last night, so I'll be heading off to bed. I'm sure someone else on this forum will be able to help you as well.

-Albatross

[FITorion]

friend bool compAlph(Student &a, Student &b);

inline bool compAlph(Student &a, Student &b){
	return a.getName().compare(b.getName())==0;
}

std::sort<std::vector<Student>::iterator>(list.begin(), list.end(), compAlph);


error:

algorithm(3191) : error C2064: term does not evaluate to a function taking 2 arguments


I also tried:
std::sort<std::vector<Student>::iterator, bool>(list.begin(), list.end(), compAlph);

but got this:

gradebook.cpp(124) : warning C4305: 'argument' : truncation from 'bool (__cdecl *)(Student &,Student &)' to 'bool'

[Athar]

You should make sure that compAlph is a static member function and the Student references in operator< and compAlph must be const.

Then call sort like this:
std::sort(list.begin(), list.end(), Student::compAlph);

Note that you can use the operators <, == and > to compare two strings.

[FITorion]

friend static bool compAlph(const Student& a, const Student& b);

inline static bool compAlph(const Student& a, const Student& b){
	return a.getName() < b.getName();  //student.h(24)
}
inline static bool operator < (const Student& a, const Student& b){
	if(a.mean() < b.mean()){ //student.h(27) 
		return true;
	}
	return false;
}

std::sort(list.begin(), list.end());

std::sort(list.begin(), list.end(), compAlph);

errors:
student.h(24) : error C2662: 'Student::getName' : cannot convert 'this' pointer from 'const Student' to 'Student &'

student.h(27) : error C2662: 'Student::mean' : cannot convert 'this' pointer from 'const Student' to 'Student &'

[FITorion]

I posted this on an other forum as well where I got this advice:

The function you are passing as your compare function must match the type declared in sort exactly. So it must be:

bool f(type elem1, type elem2);

Not a class method and not a const function.

See this example to get a better understanding.

http://msdn.microsoft.com/en-us/library/ecdecxh1%28v=VS.80%29.aspx

also, the address of a function being passed in as an argument is just "function", no & or ()s.

It works now! WooOOOOOOOHOOOOOOOOOOO!

[kempofighter]

Unfortunately you seem to have missed the point that a predicate wasn't really necessary in the first place. Defining the operator< for the object type should have been enough to allow you to call sort using the default predicate which is the std::less functor.

I'm not sure why you were ever trying to specify the template args. If you have a vector of student objects and a properly defined operator< it should be this simple. You would use a functor or predicate function in cases where you have more than one way of sorting so that you can specify the custom predicate in each call. For what you were attempting to do I am not sure why it turned out to be such a complex thread.
std::sort(students.begin(), students.end());