c++qtqsignalmapper

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?


Solution

  • 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 a (different) internal class. 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"