Construct QString from QJsonArray in Qt

Question

While trying to construct a QString from values from a QJsonArray, I get the following error:

error: passing 'const QString' as 'this' argument discards qualifiers [-fpermissive].

Dunno where I am getting it wrong in this code:

QString <CLASS_NAME>::getData(QString callerValue) {
    QString BASE_URL = "<URL>";

    QString stringToReturn = "";
    QObject::connect(manager, &QNetworkAccessManager::finished, this, [=](QNetworkReply *reply) {
      QByteArray barr = reply->readAll();
      QJsonParseError jpe;
      QJsonDocument jdoc = QJsonDocument::fromJson(barr, &jpe);
      QJsonArray synonymsArray = jdoc.array();
      foreach (const QJsonValue &jv, synonymsArray) {
        QJsonObject jo = jv.toObject();
        QString s = jo.value("<VALUE>").toString();
        stringToReturn.append(s + ", "); /* ERROR: The error above is from this line... */
      }
    }
    );
    request.setUrl(QUrl(BASE_URL + callerValue));
    manager->get(request);
    return stringToReturn;
  }

Answer 1

This is another classic "I wish the world was synchronous" problem. You can't code that way. The getData method can't be written the way you want it to be. getData done that way would block, and that's quite wasteful and can lead to interesting problems - not the last of which is horrible UX.

Depending on your application, there would be several possible fixes:

1. redo getData in implicit continuation-passing style using coroutines and co_yield - this is the future and can be done only on the most recent compilers unless you use hacks such boost coroutines.

2. redo getData in explicit continuation-passing style,

3. redo getData in lazy style with notification when data is available,

4. have an explicit state machine that deals with progress of your code.

The continuation-passing style requires the least changes. Note the other fixes too - most notably that you shouldn't be using the QNetworkAccessManager's signals: you're interested only in results to this one query, not every query! Catching QNetworkAccessManager::finished signal is only useful if you truly have a central point where all or at least most frequent requests can be handled. In such a case, it's a sensible optimization: there is overhead of several mallocs to adding the first connection to a hiterto connection-free object.

void Class::getData(const QString &urlSuffix, std::function<void(const QString &)> cont) {
  auto const urlString = QStringLiteral("URL%1").arg(urlSuffix);
  QNetworkRequest request(QUrl(urlString));
  auto *reply = m_manager.get(request);
  QObject::connect(reply, &QNetworkReply::finished, this, [=]{
    QString result;
    auto data = reply->readAll();
    QJsonParseError jpe;
    auto jdoc = QJsonDocument::fromJson(data, &jpe);
    auto const synonyms = jdoc.array();
    for (auto &value : synonyms) {
      auto object = value.toObject();
      auto s = object.value("<VALUE">).toString();
      if (!result.isEmpty())
        result.append(QLatin1String(", "))
      result.append(s);
    }
    reply->deleteLater();
    cont(result);
  });
}

The lazy style requires the code that uses getData to be restartable, and allows continuation-passing as well, as long as the continuation is connected to the signal:

class Class : public QObject {
  Q_OBJECT
  QString m_cachedData;
  QNetworkAccessManager m_manager{this};
  Q_SIGNAL void dataAvailable(const QString &);
  ...
};

QString Class::getData(const QString &urlSuffix) {
  if (!m_cachedData.isEmpty())
    return m_cachedData;

  auto const urlString = QStringLiteral("URL%1").arg(urlSuffix);
  QNetworkRequest request(QUrl(urlString));
  auto *reply = m_manager.get(request);
  QObject::connect(reply, &QNetworkReply::finished, this, [=]{
    m_cachedData.clear();
    auto data = reply->readAll();
    QJsonParseError jpe;
    auto jdoc = QJsonDocument::fromJson(data, &jpe);
    auto const synonyms = jdoc.array();
    for (auto &value : synonyms) {
      auto object = value.toObject();
      auto s = object.value("<VALUE">).toString();
      if (!m_cachedData.isEmpty())
        m_cachedData.append(QLatin1String(", "))
      m_cachedData.append(s);
    }
    reply->deleteLater();
    emit dataAvailable(m_cachedData);
  });
  return {};
}

The state machine formalizes the state progression:

class Class : public QObject {
  Q_OBJECT
  QStateMachine m_sm{this};
  QNetworkAccessManager m_manager{this};
  QPointer<QNetworkReply> m_reply;
  QState s_idle{&m_sm}, s_busy{&m_sm}, s_done{&m_sm};
  Q_SIGNAL void to_busy();
  void getData(const QString &);
  ...

};

Class::Class(QObject * parent) : QObject(parent) {
  m_sm.setInitialState(&s_idle);
  s_idle.addTransition(this, &Class::to_busy, &s_busy);
  s_done.addTransition(&s_idle);
  m_sm.start();
}

void Class::getData(const QString &urlSuffix) {
  static char const kInit[] = "initialized";
  auto const urlString = QStringLiteral("URL%1").arg(urlSuffix);
  QNetworkRequest request(QUrl(urlString));
  m_reply = m_manager.get(request);
  s_busy.addTransition(reply, &QNetworkReply::finished, &s_done);
  to_busy();
  if (!s_done.property(kInit).toBool()) {
    QObject::connect(&s_done, &QState::entered, this, [=]{
      QString result;
      auto data = m_reply->readAll();
      QJsonParseError jpe;
      auto jdoc = QJsonDocument::fromJson(data, &jpe);
      auto const synonyms = jdoc.array();
      for (auto &value : synonyms) {
        auto object = value.toObject();
        auto s = object.value("<VALUE">).toString();
        if (!result.isEmpty())
          result.append(QLatin1String(", "))
        result.append(s);
      }
      m_reply->deleteLater();
    });
    s_done.setProperty(kInit, true);
  }
}

Answer 2

Of course it would be wrong: stringToReturn is declared as a local variable in getData, It would be const if you use [=] and it will die at time of function object's call - by finished signal , if captured by reference. Lambda expression will have a dangling reference in it.That's bad place to put variable that serves as output for this function object, that variable stops to exist on return from getData.

Function object created by lambda expression here is called after getData returned. it would be called sometime when signal is fired as a result of sent request, it's an asynchronous handler. getData IS NOT the caller of lambda in this case. Caller of lambda is signal-slot system. provided that getData doesn't call lambda explicitly, we have no guarantee that function object is returned before lifespan of local storage had come to end.

Possible remebdy here is to use a field of this, if you can guarantee that this ( <CLASS_NAME> instance) is still "alive" when finished() is fired. essentially this "getData" is unable to return that value unless you would pause it until request is done (which defeats asyncronous approach).

In fact, finished would be fired when QNetworkReply::finished would be fired, QNetworkAccessManager::get just post request and returns said reply immediately. Signal is fired when you receive data (readyRead is emitted)