Serializing std::string over QDataStream

DRoscoe

My apologies in advance is this is a silly question. I thought this would be easy, but its turning out to be anything but, so maybe I am missing something.

I am converting a lot of legacy message classes to use QDataStream for serialization/deserialization. As you would expect, the legacy messages contain many non-Qt data structures, the most ubiquitous of which is std::string. In my message base class I want to define a global overload of QDataStream's streaming operators to deal with std::strings. This is what I have:

Serialization

QDataStream& operator <<(QDataStream& out, const std::string& in)
{
  out << in.c_str(); // serializing as const char*
  return out;
}

Deserialization

QDataStream& operator >>(QDataStream& in, std::string& out)
{
  quint32 size;
  in >> size;
  char* data = new char[size];
  in >> data;
  out.append(data, size);
  delete[] data;
  return in;
}

Am I doing that correctly? I supposed I could have constructed a QString in the original serialization and simply deserialized into a QString and then constructed a new std::string:

Serialization

QDataStream& operator <<(QDataStream& out, const std::string& in)
{
  QString temp(in.c_str());
  out << temp;
  return out;
}

Deserialization

QDataStream& operator >>(QDataStream& in, std::string& out)
{
  QString temp;
  in >> temp;
  out = temp.toStdString();
  return in;
}

As usual, I am looking at best practices to avoid propagating bad examples throughout my code for future developers

Chris Kawa

That's not correct. See the docs. You should not manually read the size or allocate the buffer. Qt will do that. You need only to delete it:

QDataStream& operator >>(QDataStream& in, std::string& out)
{
  char* data;
  in >> data;
  if (data)
     out = data;
  delete[] data;
  return in;
}

DRoscoe

@Chris-Kawa Ok, thanks. It was not obvious (to me) from reading the documentation that I was not responsible for allocating the memory myself, especially since I am responsible for deleting it. I used the "Serializing Qt Data Types" link to derive the solution I posted.

VRonin

Honestly, I'd go the QString way, a lot safer as you do not have to remember to handle the memory
you can use QString::fromStdString()

Buckwheat

@DRoscoe
I agree with @VRonin on this. The most maintainable is the best practice in most cases. I also look at.. "when in Rome..." or in this case Qt, use Qt and give the outside world what it wants. Unless this is some kind of real-time I/O then the miniscule amount of time for the copy to take place is worth the safety.

kshegunov

I'm with the Ronin. And one preference/advice: I'd go with QString::fromStdString always instead of through char *, it just keeps type-safety and is why the methods exist in the first place. ;)

EDIT:
And it is binary-safe! Which may be a big deal if you have a stray NULL in the string ...

DRoscoe

Thanks all!

RoyBellingan

Another option is

QDataStream& operator<<(QDataStream& out, const std::string& rhs) {
	QByteArray raw;
	raw.setRawData(rhs.data(), rhs.size());
	out << raw;
	return out;
}

Which not only is safe but will not allocate too.