[Solved] QAbstractItemModel parent() implementation

[Joel Bodenmann]

@Harb said:

I didn't understand that at all. How Foo can have ANY parent? Foo can't have parent, because it is a Top Level item in your case. Maybe I didn't understand something?

Sorry, I meant a Bar item does not know which one his parent is in the rootList. I edited the post.

I took a look at the example that you mentioned. Is there no way around creating a custom class holding the item information? I already know the structure as I can access the lists. Is there a smarter way to determine the parent?
Creating that custom class sounds like adding a lot of redundancy.
As far as I understand they use this TreeItem class because they don't have any item organization. I already have my items organized through the lists.

[Chris Kawa]

If a child (Bar) can have zero to multiple parents then it's not really a tree. It's a graph.

A tree is a structure where every node has one parent and any number of children. There is one singular node that doesn't have a parent and it's the root node.

The only way to represent a graph with a tree is to cut some edges so that the above constraints are met.

A usual underlying data representation of a node to be displayed in a treeview is something along the lines of:

struct Node {
   Node* parent;
   container<Node> children;
}

You can then extend these nodes to represent parts of your real data, e.g.

template <typename T>
struct TypeNode : Node {
   T* data;
};

and construct the structure with TypeNode<Foo> pointing to elements on your Foo's list and children TypeNode<Bar> pointing to the Bars.

If you don't want to create that abstracted structure then the easiest way is extending the Bar class so that it holds a pointer to the parent Foo.

[Joel Bodenmann]

@Chris-Kawa Thank you for your reply.
I was unclear with the parent thing, sorry. Each Bar does indeed just have one Foo parent.

Is there no way to show all children of a Foo entry in the tree without implementing the QAbstractItemModel::parent() method?

Let me explain what this is about: There is a class Library and a class Component. Library contains a list of Component pointers. In my tree view every Library is a root item. When expanding the Library I want to see all Components of that library (which are a QList<Component*> in the library).
I would really like to avoid adding a pointer to the library in the component class. I would also like to avaoid creating ab abstracted structure as you and the example linked by @Harb shows as this would somehow add redundancy.

[Chris Kawa]

@Joel-Bodenmann said:

this would somehow add redundancy

Redundancy is when some information is stored more than once. This is not the case. Here we have information missing - which library holds a given component.

You can't avoid implementing parent(). Qt needs to be able to traverse the tree up and down, not just down. That information is needed for various things, e.g. styling the tree branches. That is how the model is implemented and looking for a way around it is effort wasted. It's better to concentrate on how to cleanly implement the information required in the given model.

Since you don't want to modify existing data structures and you want to avoid duplication the only way is to make a separate structure that would hold only the missing piece, which is the child-parent relation. something as simple as QMap<Component*, Library*> would be enough. You can populate that info in some convenient place like model reset.
If you don't want to store that either then you will have to pay a runtime lookup cost of starting from the root and going down looking for a given child.

[Joel Bodenmann]

@Chris-Kawa I don't mind modifying existing data structures at all. The reason I would prefer not to store a pointer to the library in the component is because both the component and the library are "parsed from the file system". A component is a directory consisting of component metadata (JSON) and other files required for the components implementation. A library is nothing else but a directory containing many different component directories. When creating the library I simply iterate through the library directory and create a new component for each component directory using the available metadata in the JSON file which holds the component name, author and so on.

I never did anything like this before so I am happy for any suggestion on how to improve this. I would like to avoid making the library becoming anything but a directory consisting of component directories so everything is highly portable, easy to use with version control systems and so on. Eg. one can simply drag a component from one library to another. If the library itself would be a file that would be a problem because the library entries would need to be modified. Same applies to the component. If I store the information of the library the component is in in the component's metadata then that information would be modified when the component is being moved, copied or anything like that.
I could pass the pointer to the library when creating the Component instance in the actual software during run time when parsing the library/file system. I am not sure if that is a sane solution?

[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!