Sign in to follow this  

C++ "sort" stops working when move inside class

This topic is 3862 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

This works - having all the code out in the main.cpp
bool CompareMyObjects(myObject * obj1, myObject * obj2)
{
    //...
}

//...

sort(myObjects.begin(), myObjects.end(), CompareMyObjects);

But when I move the stuff inside a class;

bool myObjectsDB::CompareMyObjectsInClass(myObject * obj1, myObject * obj2)
{	
    //...
}

void myObjectsDB::Sort()
{
	sort(m_vector.begin(), m_vector.end(), CompareMyObjectsInClass);
}



I get the error; d:\Programs\Visual Studio .NET\include\algorithm(1862): error C2064: term does not evaluate to a function taking 2 arguments Can someone tell me the really simple thing that I don't know yet that will make this work?

Share this post


Link to post
Share on other sites
Move CompareMyObjectsInClass to outside of the class. I guess it wouldn't be in the class then, but it doesn't belong there in the first place.

Share this post


Link to post
Share on other sites
I would think that your main problem is that by itself the method name "CompareMyObjectsInClass" is in the abstract (i.e. since you're working with a class, it doesn't know which instance of the CompareMyObjectsInClass method....)

In myObjectsDB::Sort() try this:

sort(m_vector.begin(), m_vector.end(), this->CompareMyObjectsInClass);


Share this post


Link to post
Share on other sites
Another option is to declare CompareMyObjectsInClass static.

"Moving it into the class" makes it into a member function; non-static member functions have an extra hidden argument, the this pointer.

Right now, your CompareMyObjectsInClass function, being a non-static member function, really has THREE arguments (obj1, obj2, and this) and thus will cause an error when you try to use it in the sort template.

Share this post


Link to post
Share on other sites
Thanks Anthony - that worked! I actually had no idea about the extra hidden "this" parameter... how insidious...

DJHoy - it makes sense now that you put it that way. Sort is not part of an instantiated object...

Vorpy - thanks for the advice, the thing is, I'm looking for the "proper" way to do it too. And since my sorting function is specificaly made to operate on the objects contained in my myObjectsDB class, I figured it should belong there... am I wrong?

Share this post


Link to post
Share on other sites
The this pointer allows access to a class instances members, the compare does not need this access to do its job, this is why some have said it doesnt really belong there, and so, should be either static or global.

Share this post


Link to post
Share on other sites
I guess I am just thinking - if I want this class to be as self contained as possible, it shouldn't have such a function sitting out in the global area, it should be in its cpp. How would you usually do such a thing?

Share this post


Link to post
Share on other sites
Yeah I finally got my head around this and think static is the way to go for this one. Thanks everyone for your advice.

Share this post


Link to post
Share on other sites
The other option is (if it makes sense in your situation) is to overload operator<(), as all sorting algorithms in the standard library use this as the default.


// This should be at namespace scope. ie. outside of the class,
// but in the same namespace that it's in (if any)
bool operator<(const myObject& lhs, const myObject& rhs)
{
// Do something to compare if lhs < rhs
}

Share this post


Link to post
Share on other sites
Thankyou, yes I know of that one, but I have to sort the same objects differently depending on what sorting mode has been chosen, and I didn't want the sorting logic to exist in the data object (which is where I would have to put the operator override), since it would also somehow have to know what sorting mode the user chose, and that kind of info should stay in the database class I created, I'm pretty sure.

Share this post


Link to post
Share on other sites
Sign in to follow this