15

I'm having a lot of trouble getting my priority queue to recognize which parameter it should sort by. I've overloaded the less than operator in my custom class but it doesn't seem to use it. Here's the relevant code:

Node.h

class Node
{   
public:
    Node(...);
    ~Node();
    bool operator<(Node &aNode);
...
}

Node.cpp

#include "Node.h"
bool Node::operator<(Node &aNode)
{
    return (this->getTotalCost() < aNode.getTotalCost());
}

getTotalCost() returns an int

main.cpp

priority_queue<Node*, vector<Node*>,less<vector<Node*>::value_type> > nodesToCheck;

What am I missing and/or doing wrong?

2

2 Answers 2

24

less<vector<Node*>::value_type> Means that your comparator compares the pointers to each other, meaning your vector will be sorted by the layout in memory of the nodes.

You want to do something like this:

#include <functional>
struct DereferenceCompareNode : public std::binary_function<Node*, Node*, bool>
{
    bool operator()(const Node* lhs, const Node* rhs) const
    {
        return lhs->getTotalCost() < rhs->getTotalCost();
    }
};

// later...
priority_queue<Node*, vector<Node*>, DereferenceCompareNode> nodesToCheck;

Note that you need to be const-correct in your definition of totalCost.

EDIT: Now that C++11 is here, you don't need to inherit from std::binary_function anymore (which means you don't need to #include functional)

5
  • 1
    Out of curiosity: why define a struct with operator() rather than just a function? Commented Oct 9, 2009 at 3:43
  • 3
    You have to. You can't specialize templates with functions, just types (excluding specific circumstances). Function objects are a very important part of STL programming. A great book to read up on is Scott Meyer's Effective STL. It explains all about the STL and the best ways to take advantage of it.
    – rlbond
    Commented Oct 9, 2009 at 4:28
  • Also, I should point out that std::less<T> is also a function object (i.e., a struct with operator())
    – rlbond
    Commented Dec 21, 2009 at 15:11
  • 1
    You forgot a semicolon after struct.
    – d33tah
    Commented Mar 3, 2013 at 11:49
  • Why is this using '.' and not '->'?
    – bunnybare
    Commented Nov 10, 2014 at 7:14
16

You need to make your parameter const, because as of now you're giving it a non-cost reference, which means you might modify the object you're comparing with. (Which you aren't, and probably shouldn't).

You're not being const-correct. Your operator< doesn't make modifications to the Node, so the function should be const:

bool operator<(const Node &aNode) const;

After that, if you have trouble calling the getTotalCost() function, it's likely that it is not const as well. Mark it as const if it's not already:

int getTotalCost(void) const;

Your code is now (more) const-correct.

On a side note, binary operators are usually implemented outside the class:

class Node
{
public:
    // ...

    int getTotalCost(void) const;

    // ...
};

bool operator<(const Node& lhs, const Node& rhs)
{
    return lhs.getTotalCost() < rhs.getTotalCost();
}
1
  • Actually, I have to disagree with the definition of operator< outside the class in some instances. If it's clear cut what it's supposed to do, I don't think it's really a big deal to define it as a member. Plus it allows use of Boost.Operators.
    – rlbond
    Commented Oct 9, 2009 at 3:26

Not the answer you're looking for? Browse other questions tagged or ask your own question.