Working with your own APIs

published at 08.03.2018 14:39 by Jens Weller
Save to Instapaper Pocket

Yesterday I had the pleasure to implement a new feature in my CMS: moving a page or directory with in the website tree. Its quite easy, once you've implemented it, but it was also an interesting exercise in using code I once wrote. In Summer of 2015 I wrote this tree implementation, this old blog post also covers most of the interfaces which I needed to use now.

Tree Implementation Decisions

Back in 2015, I just got started with writing the CMS, and the tree was a needed, very basic feature. I decided against a classic, OOP Tree, where each treenode class is derived from a TreeNodeBase like class, implementing the features needed for being a tree. I decided to give static inheritance and boost::variant a try. The tree it self is generic, knows nothing about what it holds, all it knows is "I am a tree" and that it holds data in form of a variant. The types in the variant though, have no idea that they exist in a tree. In the actual implementation, a tree node contains its children, which are also tree nodes, so that there is not a separate tree and node class:

template< class ...types>
class TreeItem : public std::enable_shared_from_this< TreeItem< types... > >
{
public:
    using variant = boost::variant< types...>;
private:
    using item_t = std::shared_ptr< TreeItem<  types... > >;
    using self = TreeItem< types...>;
    variant node;
    std::vector< item_t > children;
    weak_item_t parent;
...

In the application, one instance of TreeItem servers as the root node, holding all other nodes.

Then there is another side of things: this tree needs to be represented in a QTreeView, e.g. a TreeModel like class needs to interface to the Qt model view interface. Which it self imposes some design decisions onto the tree implementation. Like that QModelIndex::internalPointer needs a pointer to the actual tree node. This implies that one has to ensure that this instance stays where this pointer points too. But I don't want to go to deep into the implementation details, as this is covered in the linked article.

The biggest decision back then was, that the tree would connect to its data via boost::variant.

Using the tree API today

Alright, so its 2018, and I want to use this API. First, yes naming things is hard, and maybe I should have done a better job with that. Some types aren't so trivially to guess. Like, item_t is a shared_ptr, maybe I should have hinted at that in the type name.

To implement the mentioned feature, to move nodes in the tree to some where else in the tree... But wait, not any node, but specifically I want to move Dir and Page types. The tree is only for these a tree, there is some other types at the tree root, holding other relevant data to be displayed in the document view of the QTreeView. But only the FixedDir Node expands into a Tree of Dir and Page types, which now should be able to be moved into any other Dir or into the level of FixedDir. Remember the implementation detail, that the classes know nothing about the tree? Dir and Page have no idea that they're in a tree, but it feels natural that they would.

So while separating the tree and its data classes was a good idea, it takes some time to get used to the actual interfaces, and also understanding, that an instance of a class held in the variant, has no connection into the tree layer. The plus point is, that these layers are separated, and so the code to move a directory or a page is the same:

auto move = [this](QModelIndex& index)
{
    auto item = static_cast< ItemTreeModel::ItemPtr >(index.internalPointer());
    auto parent = item->getParent();
    QMap< QString,DocumentTreeItem::item_t > name2dir;
    auto visit = [this,&name2dir,&item,&parent](const DocumentTreeItem::item_t& i){
        if(i.get() == item || parent == i->shared_from_this())
            return;
        if(i->type_id() == dir_typeid)
        {
            Dir* dir = i->get< Dir >();
            name2dir[QString::fromStdString(dir->getFullpath())]= i;
        }
        else if(i->type_id() == typeid(FixedDir).hash_code())
        {
            name2dir[QString("/")]= i;
        }
    };
    VisitTree<> tv(visit);
    tv.visit(item->getDocumentRoot());
    QStringList dirnames = name2dir.keys();
    auto qstring = QInputDialog::getItem(this,"CMS Dialog","Select Dir to move to",dirnames,0,false);
    if(qstring.isEmpty())
        return;
    auto moveto = name2dir[qstring];
    auto sitem = item->shared_from_this();
    parent->eraseChild(sitem);
    moveto->addChild(sitem);
};

The variant holds classes which share a common static polymorphism, implementing a method "type_id" is one of them. The actual moving of the selected node is trivial, only in preparation one has to visit the whole tree to get each directories full path, so that the user can select the one path the node should be moved into.

Lessons learned

There is some of the classic problems, like that naming things is hard. Separating the tree from the actual data was a good decision, it makes implementing new "nodes" easier, as only a few interfaces need to be supported. With C++17 out, using boost::variant does feel a bit like using legacy today, but on the other hand, not so much experience with std::variant yet. But a std::variant or a non C++17 implementation of std::variant (mparks variant e.g.) would be one of my refactoring goals today.

Also, that Dir didn't know anything about its parent or children, confused me at first. But once I've remembered the design decisions, it was clear. Also that there is no good way of retrieving a tree node instance from the type contained in the variant.

Biggest problem for me was to understand, what already existed in my API and what not. There is no way to get a map of path -> tree node pointer right now, so I wrote a local visitor for this. The blog entries I wrote while starting to work on the CMS are also today I very nice resource for myself, to understand what I was doing. So once I move on to implementing new features for the CMS, I'll continue to document them here in the blog. But for the moment, the focus is on needed changes for Meeting C++ 2018, so moving the location and ticketshop pages once the new directory for the conference exists, is like a good idea.

One more thing, this lambda then connects to the generic context menu class, which is responsible for handling the context menu in the QTreeView:

context_menu.registerAction(page_typeid,"move Page",move,this);

So, in the end, I did not need to create any new types, had only one small change to the already existing interface, which was making eraseChild public. Maybe adding a removeChild method would be better in naming. Internally it could call eraseChild with storing the childs shared_ptr first. So I'm happy with the interfaces I created back then, but once I have the time I could think about refactoring them to make them easier to understand for myself and others.

Join the Meeting C++ patreon community!
This and other posts on Meeting C++ are enabled by my supporters on patreon!