Hbi Giu Hbi Giu - 3 months ago 39
C++ Question

Best way to use QSignalMapper

I am using QSignalMapper with Qt 4.8. Now I am making network requests like below:

// start the request
QNetworkRequest request(url);
QNetworkReply* reply = networkManager->get(request);

// connect signals using QSignalMapper
QSignalMapper* signalMapper = new QSignalMapper(reply);
connect(signalMapper, SIGNAL(mapped(QObject*)),this, SLOT(onFeedRetrieved(QObject*)), Qt::UniqueConnection);
connect(reply,SIGNAL(finished()),signalMapper, SLOT(map())); // connect to the signal mapper

signalMapper->setMapping(reply, dataModel); // set the mapping (the mapping should be automatically removed when reply is destroyed)


I am doing this for each request I make, I connect the QSignalMapper with my slots each time. Is there a more elegant solution that does the same thing, perhaps using one QSignalMapper?

Answer

One simple way to do it would be to set the dataModel as a property on the reply.

Keep the network manager and other objects by value, not by pointer - the pointer is an extra indirection and completely unnecessary in most cases.

Below is a complete C++11 example that works in both Qt 4 and 5.

// https://github.com/KubaO/stackoverflown/tree/master/questions/netreply-property-38775573
#include <QtNetwork>
#include <QStringListModel> // needed for Qt 4
using DataModel = QStringListModel;
const char kDataModel[] = "dataModel";

class Worker : public QObject {
   Q_OBJECT
   QNetworkAccessManager m_manager;
   Q_SLOT void onFeedRetrieved(QNetworkReply* reply) {
      auto dataModelObject = qvariant_cast<QObject*>(reply->property(kDataModel));
      auto dataModel = qobject_cast<DataModel*>(dataModelObject);
      qDebug() << dataModel;
      emit got(reply);
   }
public:
   Worker(QObject * parent = nullptr) : QObject{parent} {
      connect(&m_manager, SIGNAL(finished(QNetworkReply*)),
              SLOT(onFeedRetrieved(QNetworkReply*)));
   }
   void newRequest(const QUrl & url, DataModel * dataModel) {
      QNetworkRequest request{url};
      auto reply = m_manager.get(request);
      reply->setProperty(kDataModel, QVariant::fromValue((QObject*)dataModel));
   }
   Q_SIGNAL void got(QNetworkReply*);
};

int main(int argc, char ** argv) {
   QCoreApplication app{argc, argv};
   DataModel model;
   Worker worker;
   worker.newRequest(
            QUrl{"http://stackoverflow.com/questions/38775573/best-way-to-use-qsignalmapper"},
            &model);
   QObject::connect(&worker, SIGNAL(got(QNetworkReply*)), &app, SLOT(quit()));
   return app.exec();
}
#include "main.moc"

You can only use the QSignalMapper if there's a 1:1 mapping between a dataModel instance and a reply. If one data model is used for multiple replies, it won't work.

If you're really concerned about allocation count, then using the property system has a bit more overhead: setting the first property on an object performs at least two allocations - an internal class, and a data segment for QMap. So that's 2N allocations. In comparison, adding a mapping to a QSignalMapper performs an amortized log(N) allocations. Otherwise, QSignalMapper is useless.

In Qt 5 using it would be outright an antipattern since you can connect to std::bind or a lambda. In any case, it'd be much nicer if QSignalMapper mapped to a QVariant.

Adding a first connection to an object also allocates the same internal class that gets allocated when you add the first property. To avoid this potential cost, you should avoid adding connections to objects you create often. It's preferable to connect once to the QNetworkManager::finished(QNetworkReply*) signal rather than connecting to every QNetworkReply::finished(). Alas, this saving is gone once you use queued connections: they, at the moment, cost an extra allocation for every argument passed to the slot. This is only a shortcoming of the current implementation, not an architectural limitation; it can be removed in a subsequent minor Qt release (if myself or someone else gets to it).

#include <QtNetwork>
#include <QStringListModel> // needed for Qt 4
using DataModel = QStringListModel;

class Worker : public QObject {
   Q_OBJECT
   QNetworkAccessManager m_manager;
   QSignalMapper m_mapper;
   // QObject::connect is not clever enough to know that QNetworkReply* is-a QObject*
   Q_SLOT void map(QNetworkReply* reply) { m_mapper.map(reply); }
   Q_SLOT void onFeedRetrieved(QObject * dataModelObject) {
      auto dataModel = qobject_cast<DataModel*>(dataModelObject);
      auto reply = qobject_cast<QNetworkReply*>(m_mapper.mapping(dataModelObject));
      qDebug() << dataModel << reply;
      emit got(reply);
   }
public:
   Worker(QObject * parent = nullptr) : QObject{parent} {
      connect(&m_manager, SIGNAL(finished(QNetworkReply*)), SLOT(map(QNetworkReply*)));
      connect(&m_mapper, SIGNAL(mapped(QObject*)), SLOT(onFeedRetrieved(QObject*)));
   }
   void newRequest(const QUrl & url, DataModel * dataModel) {
      QNetworkRequest request{url};
      auto reply = m_manager.get(request);
      // Ensure a unique mapping
      Q_ASSERT(m_mapper.mapping(dataModel) == nullptr);
      m_mapper.setMapping(reply, dataModel);
   }
   Q_SIGNAL void got(QNetworkReply*);
};

int main(int argc, char ** argv) {
   QCoreApplication app{argc, argv};
   DataModel model;
   Worker worker;
   QObject::connect(&worker, SIGNAL(got(QNetworkReply*)), &app, SLOT(quit()));
   worker.newRequest(
            QUrl{"http://stackoverflow.com/questions/38775573/best-way-to-use-qsignalmapper"},
            &model);
   return app.exec();
}
#include "main.moc"
Comments