views:

1636

answers:

4

My priority queue declared as:

std::priority_queue<*MyClass> queue;

class MyClass {
    bool operator<( const MyClass* m ) const;
}

is not sorting the items in the queue.

What is wrong? I would not like to implement a different (Compare) class.

Answer summary:

The problem is, the pointer addresses are sorted. The only way to avoid this is a class that 'compares the pointers'.

Now implemented as:

std::priority_queue<*MyClass, vector<*MyClass>, MyClass::CompStr > queue;

class MyClass {
    struct CompStr {
     bool operator()(MyClass* m1, MyClass* m2);
    }
}
+4  A: 

The operator <() you have provided will compare a MyClass object with a pointer to a MyClass object. But your queue contains only pointers (I think). You need a comparison function that takes two pointers as parameters.

All this is based on some suppositions - please post your actual code, using copy and paste.

anon
+3  A: 

Since your priority_queue contains only pointer values, it will use the default comparison operator for the pointers - this will sort them by address which is obviously not what you want. If you change the priority_queue to store the class instances by value, it will use the operator you defined. Or, you will have to provide a comparison function.

1800 INFORMATION
+4  A: 

Give the que the Compare functor ptr_less.

If you want the ptr_less to be compatible with the rest of the std library (binders, composers, ... ):

template<class T>
struct ptr_less
    : public binary_function<T, T, bool> {  
        bool operator()(const T& left, const T& right) const{
            return ((*left) <( *right));
        }
};

std::priority_queue<MyClass*, vector<MyClass*>, ptr_less<MyClass*> > que;

Otherwise you can get away with the simplified version:

struct ptr_less {
    template<class T>
    bool operator()(const T& left, const T& right) const {
        return ((*left) <( *right));
    }
};

std::priority_queue<MyClass*, vector<MyClass*>, ptr_less > que;
TimW
Can some please tell me what is wrong and why -1, this code is working and is valid C++ code.
TimW
It is making it difficult with the template operator and stuff. Still the operator between the two pointers for the specific MyClass must be implemented somewhere.
Peter Smit
So my solution is to generic? Of course you need some a way to compare two MyClass objects. Now you need a special operator to compare two MyClass pointers and I need to compare two MyClass objects something that is more natural.
TimW
It's certainly more mechanism than is needed for this one case. But in other situations it might be useful that this template works with anything that implements operator*, not just pointers.
Steve Jessop
Why does the MyClass need to know its being compared by pointers? It just has to be comparable and the algorithm needs to take pointers if it needs to
TimW
Aha, I understand your point. Thanks for helping.
Peter Smit
+2  A: 

Not sure about the priority queue stuff because I've never used it but to do a straight sort, you can do this:

class A
{
    friend struct ComparePtrToA;
public:
    A( int v=0 ):a(v){}
private:
    int a;
};

struct ComparePtrToA
{
    bool operator()(A* a1, A* a2) {return a1->a < a2->a;}
};

#include <vector>
#include <algorithm>
int _tmain(int argc, _TCHAR* argv[])
{
    vector<A*> someAs;
    someAs.push_back(new A(1));
    someAs.push_back(new A(3));
    someAs.push_back(new A(2));
    sort( someAs.begin(), someAs.end(), ComparePtrToA() );
}

Note the memory leaks, this is only an example...

Further note: This is not intended to be an implementation of priority queue! The vector is simply an example of using the functor I created to compare two objects via their pointers. Although I'm aware of what a priority queue is and roughly how it works, I have never used the STL features that implement them.

Update: I think TimW makes some valid points. I don't know why he was downvoted so much. I think my answer can be improved as follows:

class A
{
public:
    A( int v=0 ):a(v){}
    bool operator<( const A& rhs ) { return a < rhs.a; }
private:
    int a;
};

struct ComparePtrToA
{
    bool operator()(A* a1, A* a2) {return *a1 < *a2;}
};

which is cleaner (especially if you consider having a container of values rather than pointers - no further work would be necessary).

markh44
I've got to point out this is a really bad method to implement a priority queue, memory leaks and so on aside. If all you want is a sorted array, then fine, but a priority queue has a much different implementation with better performance guarantees for things like getting the first item out and inserting items.
1800 INFORMATION
I think he's just using sort() as an example, so that (a) he can present his comparator in working code rather than alone, but (b) he doesn't have to use priority_queue itself, which he says he's not familiar with...
Steve Jessop
This answer gave the right function to implement for a Comparer class. It works for my priority queue.Of course I take into account how things are going with my memory. It's not for fun that I need a priority_queue of pointers.
Peter Smit
Well as I said but since the question is about priority queues, we wouldn't want someone young and impressionable to come in and see this and think that using a sorted array is really a good method to use for a priority queue
1800 INFORMATION