How do I properly serialize and deserialize a QList class in QT using QDatastream?

VRonin

take this example:

// QList<int*> m_ownerList;
m_ownerList.append(new int(5));
m_ownerList.append(new int(3));

m_myInt1 = new  int(5);
m_myInt2 = new  int(3);
m_nonOwnerList = QList<int*>{{m_myInt1 ,m_myInt2} };

What should QDataStream& operator>>(QDataStream& , const QList<int*> ) do? free the memory already allocated or not?

JonB

@J.Hilk
But I would never want to use pointers in C# or manage memory by hand, that's (more than) half the point of using C#!?

Maybe there's a misunderstanding. I don't use C# with Qt (I use Python). I'm just familiar with C# compared to C++. I am trying to understand @VRonin's explanation of de/serializing this QList<T>, where he is saying he does it explicitly to manage pointers, when I know I would just de/serialize a list from C# without iterating the elements myself, and trying to understand why.

twodee

@VRonin Hi, thanks for the extended example. I have come up with a working solutin that looks similar to yours, could you let me know if this is a good way to deal with it?

QDataStream& operator >>(QDataStream& stream, QList<Layer*>& layers){
    layers.clear();
    int size;
    int type;

    stream>>size;

    QString name;
    QPixmap pixmap;

    Layer* layer = nullptr;
    for(int i =0; i<size; ++i){

        stream >> name >> type;
        switch (type) {
        case Layer::RASTER:
            stream >> pixmap;
            layer = new RasterLayer(name, pixmap.toImage());
            break;
        default:
            Q_UNREACHABLE();
            break;
        }
        layer->read(stream);

        layers.push_back(layer);
    }
    return stream;
}

VRonin

The very first line of your code is a memory leak. This is exactly my point. Don't handle memory inside serialising/deserialising it's not the right place

twodee

I felt wrong in doing so too as I mentioned in my original question, but I still am not sure where would I be creating the new RasterLayer in my case. Could you give me some clues on how to go about it?

VRonin

Memory owned by an object should be created and freed by a method of said object:

You can create a wrapper class around QList<Layer*> that will take care of allocating/freeing the memory inside and create safe datastream operator for this wrapper

kshegunov

@twodee said in How do I properly serialize and deserialize a QList class in QT using QDatastream?:

I felt wrong in doing so too as I mentioned in my original question, but I still am not sure where would I be creating the new RasterLayer in my case. Could you give me some clues on how to go about it?

Firstly, use the factory as you're using polymorphism, that'd be the way to go here. And secondly delegate the deserialization to the object through a virtual function; the whole reading. It's a good idea, since you're dealing with polymorphic objects, to also employ some kind of stack-based memory management here, to make sure you don't leak memory.
You can see how I did it here and draw some inspiration:
https://github.com/VSRonin/ChatExample/blob/commonlib/QtSimpleChat/chatmessage.h
https://github.com/VSRonin/ChatExample/blob/commonlib/QtSimpleChat/chatmessage.cpp

I am trying to understand @VRonin's explanation of de/serializing this QList<T>, where he is saying he does it explicitly to manage pointers, when I know I would just de/serialize a list from C# without iterating the elements myself, and trying to understand why.

'Cause when you have allocations and deallocations in different parts of the code, i.e. not declaring the objects' lifetimes clearly you're begging for trouble. C++ is lower level (thank god) than C#, so it's up to the proficiency of the programmer to actually choose the best and most versatile implementation. It may seem elitist, but we don't want the language to be something to be worked around whenever there's something falling outside of the standard set of problems to solve. In Java/C# you start by assuming the programmer is irresponsible and can't handle the problem of finding a good way to manage the memory. This may prevent a lot of errors, but also hits hard on the efficiency ...

twodee

@VRonin So, if I create a function for the wrapper around QList<Layer*> class (let's call it LayerList) that creates the correct layers depending upon the type (essentially acting as a factory), then I can go to the >> stream and instead of doing a new RasterLayer() I will call the LayerList::createLayer() method, in which case the memory is created and freed by the object owning the layers. Is my idea correct?

kshegunov

You have a two-layered problem here.

How to manage the memory
How to do (de)serialization

Start backwards, ensure you read and write the object correctly. You can do "virtual shift" operators as a syntactic sugar as well by delegating:

virtual bool Layer::serialize(QDataStream &) const;
virtual bool Layer::deserialize(QDataStream &);

// Then just delegate from the base class (need to be `friend`s to the base class):
QDataStream & operator >> (QDataStream & stream, Layer & layer)
{
    layer.deserialize(stream);
    return stream;
}

QDataStream & operator << (QDataStream & stream, const Layer & layer)
{
    layer.serialize(stream);
    return stream;
}

This should more or less take care of task 2. Don't forget to use the virtuals up the hirearchy - i.e. each derived class would call the base class' (de)serialize to get the data of the base read/written. This way you don't have code duplication and you keep the idea that each class is responsible for its own data.

As for 1), you should ideally go with some kind of clever pointer. In the example above I used QSharedData and QSharedDataPointer, but you can also use QSharedPointer (which provides external reference counting). In principle I'd always opt for the first option, as the refcount is internal to the object a reference is held to. Using QSharedDataPointer would mean also that the base class has to derive from QSharedData, just as the example I sourced above. Whenever you have that it's a breeze, as you don't have to worry when the deallocation is happening and you are left only with one thing to do - read an integer and pass it to the static function of your Layer class that'd create the correct object based on that integer and return a base pointer to it. After all that the stream operator would look really simple:

class LayerPointer; //< This is your stack-based memory management class

QDataStream & operator >> (QDataStream & stream, QList<LayerPointer> & layers)
{
    layers.clear();

    int size;
    stream >> size;
    layers.reserve(size);

    for(int i = 0; i < size; i++)  {
        stream >> type;
        LayerPointer layer = Layer::create(type); //< Factory creation
        stream >> *layer; //< The "virtual stream operator", will call the proper implementation

        layers.append(layer);
    }

    return stream;
}

twodee

@kshegunov So for 1) to work, all I have to do is a create a LayerPointer class that extends QSharedData or is there something else I would have to add? I have never dealt with this type of situation before. Also, I have already solved 2) the way you suggested, so I am currently focusing on 1).

kshegunov

See the chat example above, it's exactly what you have to do. There the ChatMessage is the base class that derives from the QSharedData and there's one typedef for the pointer: typedef QSharedDataPointer<ChatMessage> ChatMessagePointer;. Also you should read this, to have an idea what's happening behind the scenes. That's pretty much it.
Then serialization and deserialization is done in fromJson and toJson (the messages come from JSON objects), you can easily adapt that to a data stream.