[Solved] QAbstractItemModel parent() implementation

Chris Kawa

Well, another way to organize this is to extract the connection information entirely.
You could have a list of directories, a list of components and a separate structure that would hold a two-way relations between them. This way moving anything doesn't touch either the directories or components, only the relation data. Sorta like in a real file system.

Joel Bodenmann

@Chris-Kawa When I understand correctly this would make a library become just a metadata file (JSON in my case) holding nothing but (relative?) paths to the components in that library?
Therefore, everything would be completely freed from any file organization structure?

The disadvantage I see there compared to my current method is that adding a component to a library would not only be a matter of creating/moving a component directory in the library directory but actually modifying the library file.

Chris Kawa

No, I meant removing relation information out of both library and component. Something like this:

struct Library { /* ... */ }; //knows nothing about components
struct Component { /* ... */ }; //knows nothing about libraries

vector<Library> libraries;
vector<Component> components;
vector<pair<Library*, Component*>> relations.

This way adding a component to library is relations.push_back(make_pair(library, component)).

Joel Bodenmann

@Chris-Kawa Thanks, I will give this idea a thought.

With this solution I would have to create a library.json or relation.json file which would be part of the library directory as the libraries need to be distributable without every user adding the component-library relation ship manually. That is why I originally thought of making a library just becoming a file system structure. This way adding a component is just a matter of adding a component directory.

Joel Bodenmann

I have an additional question regarding the QAbstractItemModel: I have the Library which is the parent of a Component. When using QAbstractItemModel::createIndex() inside of QAbstractItemModel::index(), am I allowed to assign both, Library* and Component* to the indexes internalPointer? The internalPointer appears to be a void* pointer so this should not be a problem, right? I just want to be sure that I am not screwing up.

This way I would put a Library* in the QModelIndex when the parent is null and I would put a Component* in the QModelIndex when the parent is not null. When reading back the internalPointer I just have to do the same thing and I should be save, right?
Is there a better approach?

Chris Kawa

Yes, this is the intended usage. It is void* exactly so that you can put there whatever you want/need to identify the data associated with the index. You just need to make sure you properly "decode" that void* to whatever it is actually holding when you use it (with a proper cast).

Joel Bodenmann

@Chris-Kawa What is the proper cast? I am using static_cast().

Chris Kawa

It depends. static_cast might be perfectly ok but how do you know what to cast to (Library* or Component*)?

Joel Bodenmann

@Chris-Kawa That is my current problem. The way I do it right now is by looking at the parent index. When there is a parent then it can only be a component and I cast to Component*. When there is no parent it can only be a library and then I cast to Library*.
However, right now I cast something which is supposed to be a component to Component* but I seem to get a library out. I could not track the problem down yet. Probably I already inserted it the wrong way.

What is the proper way to do this? What do you recommend?

This is eg. my current code for rowCount():

int LibraryModel::rowCount(const QModelIndex& parent) const
{
    if (parent.isValid()) {
        const Component* component = static_cast<const Component*>(parent.internalPointer());
        if (component) {
            const Library* parentLibrary = _preferences->libraryFromComponent(component);
            if (parentLibrary) {
                return parentLibrary->components().count();
            }
        }
    } else {
        return _preferences->libraries().count();
    }

    return 0;
}

However, when I look at component in the debugger it tells me that it is a Library and I even see the member fields of a library.

Chris Kawa

Checking parent is a good shortcut way for pretty flat trees where each "level" is a separate type, so this would be a valid way in your case.
Of course it becomes impractical for deep trees(parent()->parent()->parent()->...) or trees that can have different types of nodes at the same level. Be sure to analyze how/if you intend to extend this in the future to make sure you're not gonna be in trouble later when/if you modify the hierarchy.

In more general case we circle back to the abstract Node concept that you could identify either by dynamic_casting the node to derived types or give it a void* pointer to the data and a "type" field that would identify what kind of data that is so you could static_cast to it.

Chris Kawa

About the rowCount. You fell into the naming trap. Although the parameter is named "parent" it actually refers to the node that is tested for number of children.
So here's how you should test:

if(!parent.isValid() // this is the "invisible root node"
    return /* number of libraries */;
else if(!parent.parent().isValid()) //this is a library node
    return /* cast to Library* ad return number of its components */;
else //this is a component node
   return 0; //components don't have children

Joel Bodenmann

@Chris-Kawa So to be future-proof there is no way around building an abstract node based model using an extra class (the Node / TreeItem)?

I don't mind writing the code. The only thing that is holding me back is that a user might take a component from one library and assign it to a different library during runtime. I would then have to reparse the entire library structure (every library!) to recreate the new node based model. This sounds like a lot of stuff to maintain. Listening to every possible change and creating an updated node based model sounds like a lot of computation power wasted.
When not doing the abstract node concept but just parsing the model out of the two lists which I already have this issue does not occur because the model item index is created each time based on those lists.

Joel Bodenmann

@Chris-Kawa Thank you for your follow-up on the rowCount(). I definitely interpreted that parent parameter the wrong way.
I implemented it as per your template and that part is now working.

Joel Bodenmann

In order to be future-proof I implemented the abstract node based concept as suggested by @Chris-Kawa and @Harb (in the first post).
Everything is working nicely.

Thanks for your help!