QHash with QObject based value type produces unexpected results

Pl45m4

D'oh!

Was so focused on that it part with std::max_element that I didn't even noticed.

Now I've added two more (const and not const) hashing functions which accept a pointer to MyData.

// hashing functions
inline size_t qHash(const MyData &key, size_t seed){ return qHash(key.id(), seed); }
inline size_t qHash(MyData &key, size_t seed){ return qHash(key.id(), seed); }
inline size_t qHash(const MyData *key, size_t seed){ return qHash(key->id(), seed); }
inline size_t qHash(MyData *key, size_t seed){ return qHash(key->id(), seed); }

BUT... unfortunately this doesn't seem to change anything... I'm still getting weird results in Case #4 :(
Commented the "wrong" hashing functions so that they have no impact... no changes.

Are you able to reproduce it?

Does, with the pointer hashing function, case #2 match case #4 now?

I still get something like:

##########################################################

TEST CASE 4

##########################################################
List empty -> New Data Obj starting with ID: 0
max_element in list has ID: 0
New Data Obj with next free ID created: 1
max_element in list has ID: 1
New Data Obj with next free ID created: 2
max_element in list has ID: 1
New Data Obj with next free ID created: 2
max_element in list has ID: 2
New Data Obj with next free ID created: 3
QWidget(0x20ebb5b8700) : MyData ID: 1
QWidget(0x20ebb5be1c0) : MyData ID: 3
QWidget(0x20ebb5be730) : MyData ID: 2
QWidget(0x20ebb5b87f0) : MyData ID: 0
QWidget(0x20ebb5be6a0) : MyData ID: 2
##########################################################

VRonin

What gets hashed is the key, not the value. The hash you are reading are of QWidget*

Pl45m4

@VRonin

That explains why changing the qHash function doesn't solve it. :D
So how to deal with it?

I mean, if MyData entries are not hashed, why I'm not able to compare their id?

VRonin

It depends on what you are trying to do. From what you describe it looks like what you actually want is a QHash<int,MyData*> where the key is my_daya.id().

What is not clear from above is how you are actually using the current key i.e. the QWidget*

Pl45m4

@VRonin said in QHash with QObject based value type produces unexpected results:

QHash<int,MyData*>

Why would you need this, when int ID is a member in MyData?

Actually I took the idea from the implementation of QButtonGroup.
QAbstractButton - widgets are mapped to their groupID the same way I'm trying to do... with that one difference:
As used in my "working" cases, they are mapped to the plain int ID's and not to a struct/class which contains the ID.
Like QHash<QAbstractButton *, int>, where int is the groupID.

Where I thought of:
QHash<QWidget *, MyData.id() > (makes no sense written like this, but you see what I'm trying)

I (think I) need to store a pointer to the complete class MyData, because I want to let MyData emit signals depending on the look-up results and not only store the ID.

Otherwise I would need 2-3 more maps/hashes?!
Map Widget to ID, map ID to another class (MyData), then do something and map it to my initial Widget...
That doesn't sound right/like a good design, though.

VRonin

@Pl45m4 said in QHash with QObject based value type produces unexpected results:

because I want to let MyData emit signals depending on the look-up results

If your lookup is based on QWidget * then you don't need to hash MyData because that's not what you are looking up.

If you want a hash that can search for either QWidget* or MyData* then you need a bidirectional container like KBiHash (https://invent.kde.org/frameworks/kitemmodels/-/blob/master/src/core/kbihash_p.h#L543)

Pl45m4

@VRonin said in QHash with QObject based value type produces unexpected results:

If your lookup is based on QWidget * then you don't need to hash MyData because that's not what you are looking up

So why are my results for <QWidget *, MyData*> and <QWidget *, int> different then?
In both cases I compare the ID (int).
First one via MyData::id(), second one by reading the int value from the hash directly.

From my understanding...
If in both cases the only the QWidget is hashed, then there should be no problem with the hash itself... and my logic, what I'm doing afterwards (see testFunction[Data|Int]) is completely identical.

Am I still doing something wrong there?

Has anyone tried my sample code?

Edit:

While writing this, I think I know...
std::max_element in both cases iterates the hash and seems to compare the value of all keys, returning the largest/highest value...
That means, even if I use it.value().id() to get the actual ID from MyData.... The object I'm calling this function on, is not the one with the highest ID. If I'm not mistaken, std::max_element compares the pointers to MyData and returns the highest value (~~I suspect the highest address to win~~)... since I don't have a specific operator < implemented, which tells that my intention is to compare MyData * by using MyData::id().

Will try to add a new set of operators and check the output again...

Edit_2:

Moving to C++20 so that std::max_element uses std::less might be also worth a try. Still need to implement comparison operator. Otherwise they are ordered by:

https://en.cppreference.com/w/cpp/language/operator_comparison#Pointer_total_order

VRonin

@Pl45m4 said in QHash with QObject based value type produces unexpected results:

std::max_element in both cases iterates the hash and seems to compare the value of all keys

Nope, it uses QHash<QWidget *, MyData *>::iterator::operator*() so it compares all values not all keys

@Pl45m4 said in QHash with QObject based value type produces unexpected results:

std::max_element compares the pointers to MyData and returns the highest value (I suspect the highest address to win)... since I don't have a specific operator < implemented, which tells that my intention is to compare MyData * by using MyData::id().

Bingo! that's your problem but you can just use the 3rd argument of std::max_element, no need for crazy stuff: QHash<QWidget *, MyData *>::iterator it = std::max_element(dataMapping.begin(), dataMapping.end(), [](MyData *a, MyData *b) -> bool {return a->id() < b->id();});

Pl45m4

@Pl45m4 said in QHash with QObject based value type produces unexpected results:

Will try to add a new set of operators and check the output again...

Implementing

inline bool operator<(MyData *data1, MyData *data2){ return data1->id() < data2->id(); }

results in:

main.cpp:26:13: Overloaded 'operator<' must have at least one parameter of class or enumeration type

So it's not supported by any operator and C++ standard.

And because you also cannot store QObject types by value in a container (no copy/assignment c'tor), it's never going to work :(

I guess I have to think about my design again esp. how I'm going to map the information together and how to access them :/

Maybe I get rid of the QObject inheritance and store plain data "struct", which then would be mappable again...

Thanks to anyone who contributed :)

Edit:

As the discussion was progressed further:
This turns out to be one possible solution:

@VRonin said in QHash with QObject based value type produces unexpected results:

Bingo! that's your problem but you can just use the 3rd argument of std::max_element, no need for crazy stuff: QHash<QWidget *, MyData *>::iterator it = std::max_element(dataMapping.begin(), dataMapping.end(), [](MyData *a, MyData *b) -> bool {return a->id() < b->id();});

However, will see if I re-design this whole part of my app or just use it like this... :)

VRonin

@Pl45m4 said in QHash with QObject based value type produces unexpected results:

So it's not supported by any operator and C++ standard.

Yes, overloading operators for built in types (pointers in this case) is explicitly forbidden by the standard. Hence why I use a lambda above. If you are pre-C++11 then you can use functor:

class MyDataCompare{
public:
bool operator()(MyData *data1, MyData *data2) const { return data1->id() < data2->id(); }
}

Pl45m4

@VRonin said in QHash with QObject based value type produces unexpected results:

If you are pre-C++11 then you can use functor:

Pre-C++11?!
So C++0X or C++98?!
I don't know if I want to use a C++ Standard this old.
Doesn't Qt6 need at least C++11 or something to work?

I'm still wondering what I could do, now that I know that my initial idea isn't working.

VRonin

@Pl45m4 maybe I’m missing something but I did suggest a solution above (passing a lambda as extra argument to max_element) is that not feasible for your code?

Pl45m4

@VRonin

I misread your reply.
Was looking at the functor code and thought, huh, why you need C++11 or below.
Missed that you referred with "lambda" to the code sample 1-2 posts earlier.

Yes, the lambda, in which I tell std::max_element how to compare, works.
I still take this as a chance to re-think what I'm doing and if there is a better way.

Thank you :)