Qt - What is the best way to keep a pointer to something stored in a QList?

Question

I am pretty new to Qt and C++ (been working on C# and Java all my life) and I have been reading almost everywhere that I shouldn't use raw pointers anymore (beside the ones in constructors and destructors, following RAII principle).

I am okay with it but there is a problem that I cannot easily resolve without pointers.

I'll try to explain my problem.

I am creating a QList of a custom class created by me (MyClass) that will act as a model for my application:

QList<MyClass> modelList;

This QList is updated (items added, removed or updated) by a worker thread on the basis of the information arriving from the network.

So far so good.

Now I have another thread (GUI one) that has a QList of drawable items (MyDrawableItem). Each of these items has a member that must point to one of the items in the first QList (modelList), like this:

QList<MyDrawableItem> listToDraw;

where:

class MyDrawableItem
{
private:
    MyObject *pointedObject;

    //List of many other members related to drawing
}

because everytime a timer in the GUI thread expires I have to update the drawing related members on the basis of the pointed object. Must note that it is safe to keep a pointer to members of QList as stated in the documentation:

Note: Iterators into a QLinkedList and references into heap-allocating QLists remain valid as long as the referenced items remain in the container. This is not true for iterators and references into a QVector and non-heap-allocating QLists.

Internally, QList is represented as an array of T if sizeof(T) <= sizeof(void*) and T has been declared to be either a Q_MOVABLE_TYPE or a Q_PRIMITIVE_TYPE using Q_DECLARE_TYPEINFO. Otherwise, QList is represented as an array of T* and the items are allocated on the heap.

How could this be feasible without using pointers? Or even better what should be the best way to do this kind of work?

I found three possible solutions:

1. Instead of keeping pointedObject as member of MyDrawableItem, keep the index of the pointed object in the modelList. Then on the update go look inside the QList. But what if the item in the list is removed? I could signal it but if the update happens before the slot is called?

2. Don't keep anything at all as a member and connect two signals to MyDrawableItem. One signal is updateDrawing(const MyObject&) that updates the drawing-related members and another signal is removeDrawing() that simply deletes the object. This way I don't have to worry about the syncronization between the threads because the MyDrawableItem object won't ever look at the content of the QList. In this case I could be updating the item very often with lots of signals while I only want to update the GUI once in a while (timer of 500 ms).

3. Simply use pointers.

4. Evolve and use QSharedPointers but I have never used those and I don't know if that is the best option.

I think that neither of these solutions is the perfect one so I am here asking for your help.

How do the standard views in Qt (tableview, listview, ecc.) deal with this problem?

How should I deal with it?

Please help me, I am thinking about this from yesterday and I don't want to make a mess.

Thanks a lot!

Answer 1

It is the current trend among the C++ community, especially if you look at all the CppCon conferences to discourage the use of raw pointers.

This trend is not without sold basis, as the use of std::shared_ptr and std::unique_ptr as well as std::make_unique and std::make_shared can help a lot memory management and prevent some pitfalls (like having an exception thrown during a call to a constructor).

However, there is nothing absolute in this "do not use raw pointers" trend and raw pointers can still be used and smart pointers do not solve all memory management problems.

Concerning the case of Qt, if you are dealing with QObject derived class and you properly parent your objects you are kind of safe as you have an owning hierarchy and parents QObjects will destroy their children. This is very important if you are also using QThread as QObject instances are associated with a QThread and all instances in a QObject hierachy are associated with the same thread.

Regarding your solutions:

1. Bad idea, you need to sync your list and the indexes all the time.
2. It could be an idea, but it changes the architecture of your software. Does MyObject really needs to know about MyObject ? If the answer is 'no', signal/slots seems a better solution. It might indeed require some work to solve your refresh rate issue, but this is off-topic.
3. It might look like a good idea, especially if your objects are inheriting QObject. Also you may want to use QVector<T *> and std::vector<T *> and let the QObject hierarchy deals with life span of your objects. You can listen to the destroyed() signal to know when the object are destroyed, or use QPointer. However, it does not solve the thread safeness on your consumer side, as the object might be deleted at any point in time from another thread.
4. If you cannot do 2, this is the option to go for. You just have to take care to not have cyclic dependencies or you will leak memory. However, this might be a bad idea if you want that when you delete an object in your list then it gets removed from your GUI object. If you simply keep a shared pointer, you will never know that it was removed from the list. As stated by @Felix, you can store weak pointers and convert them to shared pointers only when needed, but if you have consumers in multiple threads, then you may never delete the object. Also you have to make MyObject thread-safe as you are mutating it states from one thread and reading it from another thread. Using option 4 does change the owning model of your software: in the current state the QList owns the instances, with option 4 the ownership is shared with anyone getting a shared pointer.

My personal choice would be to replace the QList with a QVector of pointers. If your objects inherit from QObject, then I would parent all the objects in the QVector to the object that owns the QList/QVector. This is to ensure proper parenting and thread association. Then I would try option 2, and only if option 2 fails go for option 4.

Here is an exemple:

class MyClass : public QObject
{
    Q_OBJECT
public:
    explicit MyClass(QObject *parent = nullptr);

signals:
    void stateChanged(int state);
};

class Owner : public QObject
{
    Q_OBJECT
public:
    explicit Owner(QObject *parent = nullptr);
    void addObject(MyClass *obj) {
        obj->setParent(this);
        m_objects.append(obj);
    }

    void removeObject() {
        delete m_objects.takeFirst();
    }

private:
    QVector<MyClass *> m_objects;
};



class Consumer : public QObject
{
    Q_OBJECT
public:
    explicit Consumer(QObject *parent = nullptr);

public slots:
    void onStateChanged(int state) {
        m_cachedState = state;
    }

private:
    int m_cachedState = 0;
};

int main(int argc, char *argv[])
{
    QCoreApplication app(argc, argv);

    QThread thread;
    thread.start();

    Owner owner;
    Consumer consumer;
    consumer.moveToThread(&thread);

    MyClass *object = new MyClass();
    owner.addObject(object);
    QObject::connect(object, &MyClass::stateChanged, &consumer, &Consumer::onStateChanged);

    return app.exec();
}

Answer 2

An addition to @Benjamin T's answer:

If you do decide to go for option 4 and use QSharedPointer (or the std variant), you can actually control ownership somewhat using "weak pointers". Weak pointers store a reference to the object, but do not own it. Once the last "strong pointer" gets destroyed, all the weak pointers drop the reference and reset themselves to nullptr.

For your concrete example, you could use a QList<QSharedPointer<MyClass>> for the model and use QWeakPointer<MyClass> in the drawable classes. Before drawing you would create a local QSharedPointer<MyClass> in the drawing function and check if it is valid.

I would still recommend you to use 2/3, as recommended by Benjamin. Just a small addition.