Operator Overloading

[cazador7907]

I am trying to define an simple == operator for my class object. I thought, from the research and examples I'd found on line, that I had it right. Unfortunately, the nuances of C++ still escape me (I had no idea the curve was this steep!).

Anyway, If I have written the operator correct, it accepts a address reference (to a pointer) created on the heap and then does a comparison between the object and the reference based on values that I want to compare. It seems that I'm doing something (or perhaps a lot of somethings) wrong as I receive the following compile error

lvalue required as unary '&' operator.

From what I have been able to find from web searches, the compiler is complaining about the left hand values in the comparison. I THINK that it's demanding that they be pointers.

Thoughts, advice, answers, places to go to read up on this more are always greatly appreciated.

@
//Definition (from the header file)
bool operator==(Node &node);
@
//Implementation

@bool Node::operator==(Node &node)
{
return (m_sNodeName == &node.Name() &&
m_iNodeHeuristic == &node. Heuristic());
}@

[goetz]

This is the correct version:

@
bool Node::operator==(const Node &node)
{
return m_sNodeName == node.m_sNodeName &&
m_iNodeHeuristic == node.m_iNodeHeuristic;
}
@

Explanations:

• operator== wants a const reference to the right hand side (rhs) of the ==
• no need to call the getter methods, you are within the class and therefore you have access to even the private members
• you get a reference to the rhs node, so access it as a reference using . (dot). In your original code you had the & operator, which changes it to a pointer, in that case the -> operator would have been your friend. But there is no need in using a pointer, the reference does it very well too. Your version, pointer with dot, leads to a compilation error.

See the "C++ FAQ on operator overloading":http://www.parashift.com/c++-faq-lite/operator-overloading.html for some more details on the topic.

[maciej]

& is dereference operator - it gives you address of object. I don't know what is a type of m_sNodeName or m_iNodeHeuristic and what type these functions return, but you probably want values. So in return statement you shouldn't use & operator.

[Zlatomir]

Try it like this:
@bool Node::operator==(Node &node)
{
return (m_sNodeName == node.Name() && //why are you using the &
m_iNodeHeuristic == node. Heuristic()); //same here
}
@
Even if the m_sNodeName is a pointer you don't need to compare with the address of node.Name(), the addresses will be different in two objects, but the value might be the same.

[giesbert]

The solution of Volker is the right one:
But first you should check for self comparing (especially for bigger classes) and it should be const:

@
bool Node::operator==(const Node &node) const
{
if(this == &node)
return true;
return m_sNodeName == node.m_sNodeName &&
m_iNodeHeuristic == node.m_iNodeHeuristic;
}
@

[goetz]

Thanks Gerolf, now the operator overload is really complete.

[cazador7907]

Ok. I was tracking with people right up until Gerolf wrote, "first you should check for self comparing". Are you referring to a check to make sure that I'm not comparing the object with itself?

As an aside, if I have a class that inherits from Node, will the base classes == Operator function be called?

Just curious.

[Franzk]

You have to specifically tell your class to check the base class operator.

@bool SubNode::operator==(const SubNode &other) const
{
return Node::operator==(other) && ...;
}@

Operator overloading and inheritance can be a bit tricky since the operators hide each other afaik.

[Franzk]

I made an example to illustrate:
@#include <QtCore/QCoreApplication>
#include <QtDebug>

class HuuHaa {
int m_value;
public:
HuuHaa(int v) : m_value(v) { }
int value() const { return m_value; }
bool operator==(const HuuHaa &other) const
{
qDebug() << "HuuHaa";
return m_value == other.m_value;
}
};

class HurgHarg : public HuuHaa {
public:
HurgHarg(int v) : HuuHaa(v) {}
};

class Hark : public HuuHaa {
public:
Hark(int v) : HuuHaa(v) {}
bool operator==(const Hark &other) const
{
qDebug() << "Hark";
return value() == other.value();
}
};

class Murk : public HuuHaa {
public:
Murk(int v) : HuuHaa(v) {}
bool operator==(const Murk &other) const
{
qDebug() << "Murk";
return HuuHaa::operator ==(other);
}
};

int main(int argc, char *argv[])
{
QCoreApplication app(argc, argv);
HuuHaa huuhaa(1), huuhaay(1);
HurgHarg hurgharg(2), hurghargy(2);
Hark hark(3), harky(3);
Murk murk(4), murky(4);

qDebug() << (huuhaa == huuhaay);
qDebug() << (hurgharg == hurghargy);
qDebug() << (hark == harky);
qDebug() << (murk == murky);

return 0;
}
@
This results in:
@HuuHaa
true
HuuHaa
true
Hark
true
Murk
HuuHaa
true@

[goetz]

You can overcome the hiding problem with using in the derived classes:

@
using HuuHaa::operator==;
@

and declaring the operator virtual in the base class:

@
virtual bool operator==(const HuuHaa &other) const {
return m_value == other.m_value;
}
@

Then something like that also compiles, that would yield a compiler error in the original version:

@
HuuHaa hh9(9);
Hark ha9(9);
qDebug() << (ha9 == hh9);
@

[Franzk]

Ah yes, I overlooked that. However, note that it won't solve all cases. If Hark adds some data, you can have the situation where the HuuHaa comparison will state they're equal, but the Hark specific comparison won't. That is something to take into account as well when implementing operators.

Fun with operators; lots of stuff to try out :).

[cazador7907]

Aha! I am not going insane. I think that I found my problem. When i create nodes directly, the program code works brilliantly!

However, when I store pointers to nodes in a vector (and I guess any other list type object), then the comparison is being made and all this brilliant code is going to waste.

Is this a matter where - ala C# - it's necessary to cast the object as it's proper type? Something on the order of

(HuuHaa)vList[2] == DerivedHuuHaa?

[Franzk]

Well, in that case you may have a different problem. Are you comparing the node values or the pointer-to-node values?

@Node *node1 = new Node;
Node *node2 = new Node;

if (node1 == node2) {} // always false
if (*node1 == *node2) {} // may be true@

[cazador7907]

I think that I'm beginning to understand what's going on.

If I create a simple QVectory<GraphNode*> then when I create the nodes (on the heap?) and push them into the vector, I am pushing a pointer to the node. So, I would end up with a bunch of points to nodes somewhere in the Qt heap universe.

If I were to use the code below, it should work, yes? I'm setting up currNode as an iterator to the vector of GraphNode pointers and then comparing a pointer to pointer or am I still comparing apples and oranges?

@
QVector<GraphNode*>::iterator currNode;
for(currNode = graph.begin(); currNode != graph.end(); ++currNode)
{
if( currNode == nodeIn )
{
qDebug() << "Node Found.";
return true;
}
}
@

[Franzk]

I'd say you're comparing an iterator with a pointer here. In qt, the iterator is a typedef for const T *, so you're comparing a pointer to a pointer to a pointer.

Hmm, that might not be clear...

You are comparing a (pointer to a pointer) to a pointer. Basically you need to
@if (*currNode == nodeIn)@

This is where I like the foreach keyword (or Q_FOREACH macro):
@foreach (Node *currNode, nodes) {
if (currNode == nodeIn)
qDebug() << "found";
}@

[cazador7907]

Ugh. This is so odd. Once the node goes into the graph any comparison of objects within the vector just fail to run. I suppose that I could just manually check the values that I want but then that would defeat the purpose of using operators in the first place.

I've posted the offending code below. Thanks for everyone kindness and patience.

//Header

@Class Graph : public QObject
{
Q_OBJECT

private:
//Member variables
QVector<GraphNode*> graph;
int m_iNodeIndex;

bool NodeExists(GraphNode *nodeIn);

public:
//Ctors
explicit Graph(QObject *parent = 0);

void AddNode(QString name, int heuristic);

int GetNextNodexIndex();
int GraphSize();
GraphNode *GetGraphNode(int index);

signals:

public slots:

};@

//Definition
@
Graph::Graph(QObject *parent) :
QObject(parent)
{

}

bool Graph::NodeExists(GraphNode nodeIn)
{
/
QVector<GraphNode*>::iterator currNode;
for(currNode = graph.begin(); currNode != graph.end(); ++currNode)
{
if( *currNode == nodeIn )
{
qDebug() << "Node Found.";
return true;
}
}
*/

foreach( GraphNode *currNode, graph )
{
    if(currNode == nodeIn)
        return true;
}

qDebug() << "Node Not Found.";
return false;

}

void Graph::AddNode(QString name, int heuristic)
{
//Advance the Node index
m_iNodeIndex++;

//Create the new graph node
GraphNode *newNode = new GraphNode(name, heuristic, m_iNodeIndex);

if( !NodeExists(newNode) )
    graph.push_back(newNode);
else
    qDebug() << "Node Node Added.";

}

int Graph::GraphSize()
{
return graph.size();
}

GraphNode* Graph::GetGraphNode(int index)
{
return graph[index];
}
@

[Franzk]

Every time you call AddNode, you create a new node. Then you check whether the node exists by comparing the pointer to the node, rather than the node itself. The pointer to the node will never be equal to anything already in your list, since it is a newly allocated one. You need a different approach. In your case, I'd go for:

@Graph::AddNode(const QString &name, int heuristic)
{
...
if (!findNode(name, heuristic)) {
graph.push_back(new GraphNode(name, heuristic));
} else {
qDebug() << "Node already exists";
}
}

GraphNode *Graph::findNode(const QString &name, int heuristic)
{
foreach (GraphNode cur, graph) {
if (cur->name() == name && cur->heuristic() == heuristic)
return cur;
/ or if you wish:
GraphNode compareNode(name, heuristic);
foreach (GraphNode *cur, graph) {
if (*cur == compareNode)
return cur;
*/
return 0;
}@

In C and C++ you are doing your own memory management. For every new you have do a delete in some way to prevent memory leaks.

In your version:
@bool Graph::NodeExists(GraphNode *nodeIn)
{
foreach( GraphNode *currNode, graph )
{
if(*currNode == *nodeIn) // changed
return true;
}

qDebug() << "Node Not Found.";
return false;

}

void Graph::AddNode(QString name, int heuristic)
{
//Advance the Node index
m_iNodeIndex++;

//Create the new graph node
GraphNode *newNode = new GraphNode(name, heuristic, m_iNodeIndex);

if( !NodeExists(newNode) ) {
    graph.push_back(newNode);
} else {
    qDebug() << "Node Not Added.";
    delete newNode; // changed
}

}@

[goetz]

These are pointer/reference bascis. It depends, on what you want to do.

Object identity - this would be true if two objects have the same address in the memory. To achieve that you compare pointers.

@
Node *n1 = new Node(1);
Node *n2 = new Node(2);
Node *n3 = n2;

qDebug() << n1 == n2; // false different addresses
qDebug() << n1 == n3; // false different addresses
qDebug() << n2 == n3; // true same addresses
@

If you are interested if two objects are equal in the sense of containing the same values, you must compare dereferenced pointers or references/values:

@
Node n1(1);
Node n2(2);
Node n3 = n2;

// the adresses of the nodes:
Node *np1 = &n1;
Node *np2 = &n2;
Node *np3 = &n3;

qDebug() << n1 == n2; // false, 1 != 2
qDebug() << n1 == n3; // false, 1 != 2
qDebug() << n2 == n3; // true, 2 == 2

qDebug() << np1 == np2; // false
qDebug() << np1 == np3; // false
qDebug() << np2 == np3; // false
@

[Franzk]

His code suggests he wants to know whether a node with name and heuristic already exists.