Iterator as a member: Tree/Graph-like structure

[JonB]

@Pl45m4 said in Iterator as a member: Tree/Graph-like structure:

When using QHash and I want to iterate straight through, I have to search for "next" ID manually...

Yes that's exactly as @Christian-Ehrlicher said, because QHash is not ordered. Forget about it!

on the other hand, when using a QMap, first I need to order them by MyTask::id (thanks @Christian-Ehrlicher ) but then the insertations/deletions are quite messy and complicated :)

Why are insertions/deletions a problem?? (I haven't looked at your code!.)

[Pl45m4]

@JonB said in Iterator as a member: Tree/Graph-like structure:

Why are insertions/deletions a problem?? (I haven't looked at your code!.)

Because I still have to "search" for the next ID in order (as it's not always lastID + 1) that would make this more like being close to O(n^2) than O(n) or even O(1), right? Or am I missing something?!

Also I asked initially if it's "ok" to have a global (member) iterator around instead of keeping some index (which might change) or a direct pointer to the item itself (= what also is stored in the container).

[JonB]

@Pl45m4 said in Iterator as a member: Tree/Graph-like structure:

Because I still have to "search" for the next ID in order (as it's not always lastID + 1) that would make this more like being close to O(n^2) than O(n) or even O(1), right? Or am I missing something?!

I don't understand this at all. I don't even know whether you mean doing this at insertion, deletion or search-for-next time, but in all cases my searches will be O(log(n)). And that applies when looking for key 10 whether it finds it or returns where it ought to be if it does not exist.

Since @Christian-Ehrlicher said I may not have made it 100% clear. All in all I would probably use

QMap<int, Task *> map;
map.insert(task->id, task);

You can use QMap's lower/upperBound() to find where you got to/where to start from next, and this will work even if the previously noted id number no longer exists (e.g. it has been deleted). [Go read docs about what these return if the key you ask for does not exist, you do not have to explicitly find last + 1 in existence, this is the bit you are not understanding.] And you can assume that will be O(log(n)) because it knows the key search is ordered, unless it is brain-damaged, which I imagine it is not :) Which is all similar for QMap as it would be if you wrote your own ordered vector for binary search or red-black tree. (I am guessing QMap is some kind of red-black tree?)

Also I asked initially if it's "ok" to have a global (member) iterator around instead of keeping some index (which might change) or a direct pointer to the item itself (= what also is stored in the container).

I think this was covered in @Christian-Ehrlicher's initial answer, where an iterator is no better than a pointer to keep around in the case where that item may have been deleted.

[Pl45m4]

@JonB said in Iterator as a member: Tree/Graph-like structure:

Since @Christian-Ehrlicher said I may not have made it 100% clear. All in all I would probably use

QMap<int, Task *> map;
map.insert(task->id, task);

This makes totally sense and I also got what @Christian-Ehrlicher wrote above about a custom "key", but is it really a good idea to map MyTask to members of itself?!
Is there no other magical way to do it?
That stopped me from going over this approach in my head any further.
Because the "fun" thing is that (with a QMap) I currently wouldn't know what to put as value otherwise, when using a key like @Christian-Ehrlicher described before and correctly :))

I have

// my current approach task container
QList<MyTask*> taskList;

// (not included in my example and not relevant for looping the tasks)
// where "int" equals a valid MyTask::id
QHash<MyWidget *, int> taskWidgetMap;

so the information what Task has which ID is already stored in MyTask class

To get somewhere with this, I will try @Christian-Ehrlicher 's approach and your MyTask <--> MyTask::id mapping now and see how it integrates into the rest. ;-)

Btw: Now I've read through QSet<T> more carefully and figured out that my initial thought (in my head without specifying any data structure) was about something like an ordered (ideally hash-based) one-dimensional structure (= "list", no key-value dict).... which does not existing in this form :)

So yeah, I will report back later ;-)

Besides the data struture mess, have you tried my example @JonB @Christian-Ehrlicher ? What do you think? :)

Highly appreciate all your input and the discussion here :)

[JonB]

@Pl45m4

• Christian's approach of defining a < operator for the Task struct itself is required if and only if you wish to have a Task * as the key for the QMap. Which is what he says you had stated initially.
• But I don't see why you would want or need that (my Task * is the value, not the key). I just use an int as the key and pass task->id for that at map insert time.

Up to you.

[Pl45m4]

@JonB said in Iterator as a member: Tree/Graph-like structure:

I just use an int as the key and pass task->id for that at map insert time

Yeah that makes sense... but my concern is/was that I have some redundancy. MyTask::id = key AND also already stored in MyTask (and accessible via something like value().id)...

Will try it out.

[JonB]

@Pl45m4
You are saving/losing an int for the key. But even another way to use a QMap you have to provide some key to go with a value. If you use Task * as the key that costs a pointer (even if you also store Task * as the value too) which is actually bigger than an int. Plus your QMap actually goes wrong if you go change what the key pointer points to, or the id inside that, if you change the id in the Task your QMap won't rearrange itself!

Many times we do key-value pairs like this. I could be wrong, but when, say, you have a database table with a primary (or unique) key/index I don't think that stores a "pointer to" its value somewhere in the row for its data, I think it copies the value to the index and then keeps that in sync if the row changes.

But if you are happier with the key as a Task * and override the < operator that is fine too.

[Pl45m4]

@Christian-Ehrlicher said in Iterator as a member: Tree/Graph-like structure:

simply replace QMap<MyTask *, whatever> with QMap<Key, whatever> and provide a operator<() for the key (I was wrong above - you don't have to provide a qHash() but a operator <() for a QMap)

struct Key {
  MyTask *task;
  bool operator <(const Key &o) const
  {
    return  task->id < o.task->id;
  }
};

 QMap<Key, something> myMap;

One more thing @Christian-Ehrlicher :
Is there a reason why you picked an extra struct for the Key instead of using the MyTask::operator < directly?

[Christian Ehrlicher]

@Pl45m4 said in Iterator as a member: Tree/Graph-like structure:

instead of using the MyTask::operator < directly?

Because this operator would not be used by a QMap<MyTask*, ...> as you store a pointer, not a value.

[Pl45m4]

@Christian-Ehrlicher

Ah I see, thanks.