Serializing QMap<qint8, Claass*> with QDataStream

[simex]

I have a problem serializing and reading back a QMap that holds a collection of my class called PID.

In this class I implemented operators << and >> as below, with the help of some good fellas here in this forum.


QDataStream &operator <<(QDataStream &out, const PID &pid)
{
out << pid.getKp() << pid.getKi() << pid.getKd();
return out;
}


QDataStream &operator >>(QDataStream &in, PID *pid)
{
double kp;
double ki;
double kd;
in >> kp >> ki >> kd;
pid = new PID();
pid->setKp(kp);
pid->setKi(ki);
pid->setKd(kd);
return in;
}

Latter the map is serialized like this


PID pid;
pid.setKp(34);
pid.setKi(45);
pid.setKd(54);
QString path = appDir.path() + QDir::separator()
+ name + TXT_FILE_EXT;
QFile pidFile(path);
pidFile.open(QIODevice::ReadWrite);
QDataStream stream(&pidFile);
QMap<quint8, PID*> pidList;
stream << pidList;

pidList.clear();
stream.device()->reset();

stream >> pidList;

qDebug() << "Pid map size: " << pidList.size() << " File Size: "<< pidFile.size();
pidFile.close();

when I run this I get the same "Pid map size: 0 File Size: 8 " even when the map's size is changed in the code. Any Idea how I can get to my goal of serializing the collection of PIDs to a file and read back?
Thanks.

[yeye_olive]

According to QMap's documentation, you can only serialize it to/unserialize it from QDataStream if both key and value types themselves are serializable. The value type of your QMap, PID *, is a plain pointer; I have no idea how it gets serialized to QDataStream, but I doubt it is what you expect (it sure does not call QDataStream &operator <<(QDataStream &out, const PID &pid)). As for deserialization, I can only wonder why the program does not crash randomly when it calls your QDataStream &operator >>(QDataStream &in, PID *pid) on an uninitialized PID *.

Just think about it for a minute. If your QMap holds PID * values, then I suppose that you allocate PID instances somewhere, then populate the map with pointers to them. In order to deserialize such a map, you would need to allocate PID instances. Your code does not do that.

[simex]

Thank you yeye_olive.
I was trying to go with pointers because then otherwise I would have to overload a copy constructor and an assignment operator, and by doing so risk loosing the automatic garbage collection feature that comes with QObject. (but my fear could be void as my understanding of Qt is not that big).

Anyway I tried to write the operator QDataStream &operator <<(QDataStream &out, const PID &pid) in such manner so it also takes a pointer then at least the written/read types are the same;


QDataStream &operator <<(QDataStream &out, const PID *pid)
{
out << pid->getKp() << pid->getKi() << pid->getKd();
return out;
}

it didn't give an error, and the map size was not zero anymore but the values were one big number that was not the originally saved once.

So I changed the operators, added a copy constructor and assignment operator to the PID type and now it works like a charm. But I still wonder what would have been the way to go with pointers? is it even possible to do so?


PID::PID(const PID &other)
{
kp = other.getKp();
ki = other.getKi();
kd = other.getKd();
}

PID &PID::operator=(const PID &other)
{
kp = other.getKp();
ki = other.getKi();
kd = other.getKd();
}

QDataStream &operator <<(QDataStream &out, const PID &pid)
{
out << pid.getKp() << pid.getKi() << pid.getKd();
return out;
}


QDataStream &operator >>(QDataStream &in, PID &pid)
{
double kp;
double ki;
double kd;
in >> kp;
in >> ki;
in >> kd;
pid.setKp(kp);
pid.setKi(ki);
pid.setKd(kd);
return in;
}


Any suggestion is welcome again.
I am sure I didn't write the assignment operator and the copy constructor very well and may cause me trouble later on when I try to use them to populate a view.
Once again thanks.

[yeye_olive]

A deserialization operator takes a reference to the object whose value shall be overwritten. For instance your operator:

QDataStream &operator >>(QDataStream &in, PID &pid)
deserializes a PID value and overwrites pid with it.

With pointers, you would have needed a deserialization operator such as (notice the reference to pointer):

QDataStream &operator >>(QDataStream &in, PID *&ppid) {
double kp;
double ki;
double kd;
in >> kp;
in >> ki;
in >> kd;
ppid = new PID(kp, ki, kd);
return in;
}
but you can see how fragile that would have been:

the operator throws away the old value of ppid without calling delete on it first, potentially leaking memory, but it cannot do so because someone may call this operator on an uninitialized PID * (just like we do with double kp, ki, and kd in the code above);
the operator allocates a PID, but nothing tracks this allocation; if this operator were to be called in the middle of a complex operation which subsequently throws an exception, chances are that memory would be leaked.


There is a reason why we do not (de)serialize arbitrary naked pointers: they are just dumb containers for addresses without any enforced semantics for memory allocation and deallocation.

A much better solution would be to wrap a PID * in a smart pointer class with the desired semantics, and write (de)serialization operators for that class.

About your PID::operator=(). You forgot a "return *this;". Also, if all your operator does is field-by-field assignment, then you can remove it and leave the compiler generate the same thing by default. Similarly for your copy constructor if it only does field-by-field copy initialization.

[simex]

Thank you for breaking down this issues. :) I'll put this to an effect.