QList<mypointer *>::contains and operator== issue

[jamo_]

Hi,

I'm having some issues with QList<mypointer *>::contains and operator==.

It is probably easiest to explain this with code - am basically attempting to compare a pointer with one in a list.



QList <MyPointerBase *> p1;
QList <MyPointerInherited *> p2; //already instantiated

for (int i = 0 ; i < p2.size() ; i++) {
MyPointer *mp = new MyPointerBase(p2[i]);
if (p1.isEmpty() || !p1.contains(mp))
{
p1 << mp;
}
}


However the !p1.contains(mp) always returns true, I suppose because it is comparing two pointers?

I have overloaded operator== for the two classes as well using both methods

Surely the second one should be used for the comparison? I have dbugged this but the code never makes it here..



bool operator== (MyPointerBase &pb1, MyPointerBase &pb2) {
return (pb1.id == pb2.id);
}

bool MyPointerBase::operator== (const MyPointerBase *other) const {
return (this->id == other->id);
}


Aside from creating my own version of "contains" can anyone see anything obvious I am doing wrong?

Thanks

:-)

[minirop]

If I not mistaken, bool MyPointerBase::operator== (const MyPointerBase *other) const { is never called because you compare pointers.

edit : and operator== takes (or should take) only references. wiki link (http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B#Comparison_operators.2F Relational_operators)

[john_god]

What a mess you've got there :)
If I understood it correctly, you want to check all elements from p2 and if they are not already in p1, you want to append them ?


for(int i=0;i<p2.size();i++)
{
if (!p1.contains(p2.at(i) )
{
MyPointerBase *mp = new MyPointerBase ();
*mp=*p2.at(i);
p1.append(mp);
}
}


I have dbugged this but the code never makes it here..
Because in your code you not using the operator == (at least in the code you show).
I have not tested my code, dont now if it will work :cool:

[jamo_]

Hey,

Thanks. Yes it is a bit ugly but the code is more complicated than above.. The classes have unique id's and so the == overload just checked that rather than everything in the class.

I found another way to do it that was a bit cleaner (well I hope) by iterating over a QMap of these id's.

Thanks though!

:-)

[d_stranz]

But do you realize what was wrong with your original code?


MyPointer *mp = new MyPointerBase(p2[i]);

In this line you create a new instance of MyPointerBase using a pointer to another instance of MyPointer. I hope you understand that this is not a copy constructor.


if (p1.isEmpty() || !p1.contains(mp))

In the very next line you ask the QList if it contains the pointer you just created. Of course it doesn't - you just made it, and have not inserted it into the container yet.

I think you have some very basic confusion between object instances and pointers to object instances. Your QList is a container of pointers and therefore any comparison is between the pointers themselves, not the object instances those pointers point to. So your class operator==() will never be called during any QList method that compares members of the list.

Even so, your attempt to overload operator==() is just off track.



// This really isn't a class member at all.
// This operator would be invoked through a call like
// if ( *mp1 == *mp2 )
bool operator== (MyPointerBase &pb1, MyPointerBase &pb2) {
return (pb1.id == pb2.id);
}

// This one is a class member, but it's wrong.
// This one would get called through code like
// if ( *mp1 == mp2 )
// or
// if ( mp1->operator==( mp2 ) )
// which is downright weird. How would your replacement ever understand code like that?

bool MyPointerBase::operator== (const MyPointerBase *other) const {
return (this->id == other->id);
}


If you want a class operator==(), then you write this:



bool MyPointerBase::operator==( const MyPointerBase & other ) const
{
return id == other.id;
}


and call it using pointers to class instances like this:



// In ALL cases mp1 and mp2 must be non-NULL
if ( *mp1 == *mp2 )
{
// ...
}
// or

if ( mp1->operator==( *mp2 ) )
{
// ...
}

// or

MyClass & mp1Ref = *mp1;
MyClass & mp2Ref = *mp2;
if ( mp1Ref == mp2Ref )
{
// ...
}

[jamo_]

Perfect. Thanks for the pointers.. Finally got it to work!

I think I understand it now - definitely now it isn't 1am!