views:

153

answers:

6

Hello, I have seen other people questions but found none that applied to what I'm trying to achieve here.

I'm trying to sort Entities via my EntityManager class using std::sort and a std::vector<Entity *>

/*Entity.h*/
class Entity
{
public:
 float x,y;
};

struct compareByX{
 bool operator()(const GameEntity &a, const GameEntity &b)
 {
  return (a.x < b.x);
 }
};   

/*Class EntityManager that uses  Entitiy*/

typedef std::vector<Entity *> ENTITY_VECTOR; //Entity reference vector

class EntityManager: public Entity
{
private:
 ENTITY_VECTOR managedEntities;

public:
 void sortEntitiesX();
};

void EntityManager::sortEntitiesX()
{

 /*perform sorting of the entitiesList by their X value*/
 compareByX comparer;

 std::sort(entityList.begin(), entityList.end(), comparer);
}

I'm getting a dozen of errors like

: error: no match for call to '(compareByX) (GameEntity* const&, GameEntity* const&)'
: note: candidates are: bool compareByX::operator()(const GameEntity&, const GameEntity&)

I'm not sure but ENTITY_VECTOR is std::vector<Entity *> , and I don't know if that could be the problem when using the compareByX functor ?

I'm pretty new to C++, so any kind of help is welcome.

+1  A: 

I did see this question, recently, though....

The answer was something in the way of: the function provided to sort should not be a member-function of something. Meaning: it should be a static function, or a free function. In case you declare it a static function, you should still precede it by Entity::compareByX in order to name it correctly.

If you define the order in the class itself, you can, as aJ already said, use a function adapter mem_fun or mem_fun_ref to pour it into a 'free' functor object.

If you want an Entity object to do the comparison, you should provide sort with an object (called a functor or comparator in this case):

struct EntityComp {
  bool operator()( const GameEntity& a, const GameEntity& b ) const { 
    return a.x < b.x;
  }
}


...
std::sort( v.begin(), v.end(), EntityComp() );
xtofl
I think the correct syntax is: `std::sort( v.begin(), v.end(), EntityComp );`
Joy Dutta
I am trying to do: std::sort(entityList.begin(), entityList.end(), std::mem_fun( ,but I get error: no matching function for call to 'Entity::compareByX()' . is Entity::compareByX, the correct way to pass that argument ?
Mr.Gando
@Joy: `EntityComp` will not result in an object, neither in a function. `EntityComp()` will result in a temporary functor object, passed to the `sort` function. You may as well construct the `EntityComp` upfront, like `EntityComp e; sort( .... e );`.
xtofl
@Mr.Gando: Indeed: ``.If you're making the function `static`, there is no need either to create a class for it - just make it a free function.
xtofl
by "free function" you mean to use something like this ? :struct compareByX{ bool operator()(const GameEntity }}
Mr.Gando
I meant something like `bool compareByX( const GE }`. What you asked is called a functor.
xtofl
+1  A: 

I believe compareByX should be a static member or lake a look here

Elalfer
+2  A: 

A functor is a class that defines operator() so an object of that class can be "invoked" with the same syntax as calling a function:

struct functor { 
   bool operator()(Entity const &a, Entity const &b) {
       return a.x < b.x;
   }
};

If you want that as a member of your Entity class, you'd use a nested class:

class Entity { 
    float x;
public:
    friend class byX;
    class byX {
        bool operator()(Entity const &a, Entity const &b) { 
            return a.x < b.x;
        }
    };
};

Then your sort would look something like this:

std::sort(ManagedEndities.begin(), ManagedEntities.end(), Entity::byX());

Alternatively, if you usually sort Entities by X, you could define operator< for Entity:

class Entity { 
     float x;
public:
     bool operator<(Entity const &other) { 
         return x < other.x;
     }
};

In this case, your use of sort would be a bit simpler:

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

Creating the comparison function as a normal member function of the Entity class, however, will lead to a sort invocation that's pretty ugly -- it'll usually need something like std::mem_fun_ref to do the job; it's sufficiently ugly that I'd generally avoid it for real code.

Jerry Coffin
Jerry, I believe the () at the end of the functor (in std::sort(..., ..., Entity::byX());)are not necesary - that would pass the return type (bool) rather than the functor object itself.
Fox
If you would take the "struct functor" approach, where would you place that struct ? in Entity.h for example ? ( just to keep things in order I mean )
Mr.Gando
@Fox: not so -- byX is just the name of a type. byX() creates a temporary object of that type. std::sort will then invoke operator() on that object.
Jerry Coffin
@Mr. Gando: where to put the comparator depends on usage. If it's basically part of Entity, and only ever used to sort Entity objects, then I'd probably embed it in Entity's definition, in Entity.h. In some cases, the same comparator can be used on a number of different types, in which case it assumes its own identity, rather than being part of something else.
Jerry Coffin
@Fox: funny, Joy asked the same thing when reading my answer :)
xtofl
A: 

try this..

 class CompareByX
 {
   operator ()(const GameEntity &a, const GameEntity &b) { ... };
 };

 ...
 std::sort( this->begin(), this->end(), CompareByX);

In a nutshell, a functor is a function object - the STL looks specifically for an operator () that takes in the two parameters I've specified. If you're new to C++, I suggest you look up operators and functors - they're pretty handy even outside STL.

Edit: Jerry's answer is better, and more comprehensive.

Fox
Your answer is so simple, and so almost correct... You should create an object as comparator.
xtofl
+1  A: 

In the light of 'what you're trying to achieve', I may do another guess... You want to be able to specify whether to compare your objects by their GameEntity::x member, or by their GameEntity::y member.

The easiest way would be to, as you did, specify a functor for each member:

struct CompareX {
   bool operator()( const GameEntity& a, const GameEntity& b ) const {
      return a.x < b.x;
   }
};

struct CompareY {
   bool operator()( const GameEntity& a, const GameEntity& b ) const {
      return a.y < b.y;
   }
};

CompareX compx; // create a compare object
std::sort( v.begin(), v.end(), compx );

The 'flexible' yet more cumbersome way would be to create a template functor:

#include <iostream>

using namespace std;

// a mockup of your class
struct GameEntity { float x, y, z; };

// just to be able to print it...
ostream& operator<<( ostream& o, const GameEntity& g ) {
  return o << "(" << g.x << ", " << g.y << ", " << g.z << ")";
}

// cumbersome starts here...
typedef float (GameEntity::*membervar);

// a 'generic' float-member comparator
template< membervar m > struct CompareBy {
   bool operator()( const GameEntity& a, const GameEntity& b ) const {
      return a.*m < b.*m ;
   }
};

// example code
int main() {
   using namespace std;
   GameEntity v[] = { {1,0,0}, {2,0,1}, {3,-1,2} };
   GameEntity* vend = v + sizeof(v)/sizeof(v[0]);

   sort( v, vend, CompareBy< &GameEntity::x >() );
   copy( v, vend, ostream_iterator<GameEntity>( cout, "\n" ) );
}
xtofl
+2  A: 

And a third one comes in... After you edited you question, still one open topic: your comparator takes a const & to the GameEntity class. It should, in order to work with the values of the vector<GameEntity*>, take const GameEntity* arguments instead.

xtofl
I am surprised no-one else noted it.
Georg Fritzsche