0

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;
  }
4
  • 1
    I think you need to pass your stringToReturn variable to lambda (capture) by reference and not by copy, i.e. use [&] instead of [=], because it's const in the lambda body. Commented Apr 5, 2018 at 6:51
  • Doing so crashes the app just a few seconds after the GuI appears :( Commented Apr 5, 2018 at 7:14
  • You need to investigate why and where it crashes. For example by reading the stack trace or debugging. Commented Apr 5, 2018 at 7:17
  • of course it would: stringToReturn is a local variable, lambda expression will have a dangling reference in it. Commented Apr 5, 2018 at 7:34

2 Answers 2

2

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);
  }
}
Sign up to request clarification or add additional context in comments.

3 Comments

oh, did you had working examples handy? That's quite detailed implementation. And yeah, wish to ho synchronous is common sin. Personally i prefer state machine approach. What's UX stands for, though? About this api I always was worried if a race condition would happen - what if signsl is fired before we called connect (wery intensive network exchange). Is that possible scenario?
It is unclear how to delete reply in first example
@Swift The races don't happen. The implementation prevents it. The examples are written from memory. You might note that they are very short.
1

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)

4 Comments

Hm, but getData() function returns a copy of stringToReturn and not a reference to it. So getData() defines a variable, lambda modifies it and its copy returned back to the caller - that's absolutely ok.
What’s a possible remedy, then?
@ vahancho lambda here is called after getData returned. it would be called sometime when signal is fired (as 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.
@vahancho getData would return empty string,. In theory if readyRead will be called before getData return, that can be different, but it's not guaranteed by presented code. It would so for two reasons: lambda can't modify copy-captured objects. 2) lambda isn't called during lifetime of scope of capture.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.