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

[twodee]

@VRonin Hi, thanks for responding. I am getting this error which I usually can avoid using a const somewhere: error: passing ‘const RasterLayer’ as ‘this’ argument discards qualifiers [-fpermissive] : setName(name);

As you can see in the layer.h, it is an inline method inline void setName(QString &name) { _name = name; } , what should I be doing in this case?

[VRonin]

Looks like you are deserialising on a const method. What is the code around that call?

[twodee]

This post is deleted!

[twodee]

@VRonin I figured the serialization out, now I have to do something like out << *layer; to get it serialized. But how will that allow me to serialize a QList<Layer*> to be serialized as a whole?

[JonB]

@twodee
Read through threadin this forum https://forum.qt.io/topic/58701/how-to-serialize-deserialize-a-qlist-myclass

As per http://doc.qt.io/qt-5/datastreamformat.html, QList<T> can be de/serialized. You need to provide the de/serialization for your class T, which is what you have been working on here.

[twodee]

This post is deleted!

[twodee]

As the post said it's about one line with the operator new, could you tell me what that line is?

 QList<Layer*> layers = paintWidget->getItems(); 
out << layers;

This doesn't seem to do the trick. I have been scratching my head for too long, sorry if I am being stupid.

Also for some reason overloading QDataStream& operator<<(QDataStream& ds, const Layer *layer) doesn't work anymore, which did work earlier.

[VRonin]

out << qint32(layers.size());
for(Layer* layer : qAsConst(layers))
out << qint32(layer->type()) << *layer;

---

qint32 size;
qint32 type;
in >> size;
while(size-- >0 ){
in >> type;
Layer* layer = nullptr;
switch(type){
case Raster:
layer = new RasterLayer;
break;
default:
Q_UNREACHABLE();
}
in >> *layer;
}

[JonB]

@VRonin
I don't want to muddy the waters for the OP here, but may I ask: why do you explicitly serialize the layers list yourself in a loop? I thought that the point of the link I mentioned, http://doc.qt.io/qt-5/datastreamformat.html, is that it shows:

The QDataStream allows you to serialize some of the Qt data types. The table below lists the data types that QDataStream can serialize

QList<T>	
    The number of items (quint32)
    The items (T)

so why can't you just out << layers ?

Is this because you want to know where layer->type() is in the serialization, so that you can look at it during deserialization and do your own newing? If there were no sub-classing going on in the list elements then you wouldn't need to do that and could just do the list directly? Could it then just be deserialized with in >> layers? Or will deserializing never do any newing of elements for you?

[VRonin]

@JonB That would imply having datastream operators acting on pointers and that's dangerous:

• What if you pass a null pointer
• What if you pass a dangling pointer
• Who own the memory allocated by the pointer? the operator?

[JonB]

@VRonin
So are you saying: "Yes, you can de/serialize QList<T> directly as per that link, but while that's fine for simple types it's not suitable for pointers"?

I may be confusing myself. In my C# we don't have "pointers" and we just de/serialize lists directly without a care. Deserializing does whatever newing is necessary behind the scenes.

[J.Hilk]

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

I may be confusing myself. In my C# we don't have "pointers" and we just de/serialize lists directly without a care.

I beg to differ!

you can use pointers in c# and manage your memory by hand, but you have to explicitly tell the compiler to allow it with -unsafe, IIRC

Quick google search:
https://www.tutorialspoint.com/csharp/csharp_unsafe_codes.htm

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