Code cleanup and tweaking (partially driven by clang-tidy)

Mainly it's about const-ification (in particular, passing const-refs instead of values) and deleting unneeded declarations/#includes. Since the changes alter the external interface, this is submitted as a PR for peer review.

One of unneeded declarations/definitions is a virtual destructor in BaseJob descendants. Since a job object should be deleted through QObject::deleteLater() anyway (and it's the only correct way of disposing of the object), all deletions will call the stack of destructors through virtual QObject::~QObject(). Therefore even BaseJob could get on with a non-virtual destructor but for the sake of clarity BaseJob::~BaseJob() is still declared virtual.

34 files changed, 166 insertions, 228 deletions

diff --git a/connection.cpp b/connection.cpp
index 0fc23f2f..8d5ff2fa 100644
--- a/connection.cpp
+++ b/connection.cpp
@@ -39,20 +39,20 @@ using namespace QMatrixClient;
 class Connection::Private
 {
     public:
-        explicit Private(QUrl serverUrl)
+        explicit Private(const QUrl& serverUrl)
             : q(nullptr)
             , data(new ConnectionData(serverUrl))
-            , isConnected(false)
             , syncJob(nullptr)
         { }
-        Private(Private&) = delete;
+        Q_DISABLE_COPY(Private)
+        Private(Private&&) = delete;
+        Private operator=(Private&&) = delete;
         ~Private() { delete data; }
 
         Connection* q;
         ConnectionData* data;
         QHash<QString, Room*> roomMap;
         QHash<QString, User*> userMap;
-        bool isConnected;
         QString username;
         QString password;
         QString userId;
@@ -60,7 +60,7 @@ class Connection::Private
         SyncJob* syncJob;
 };
 
-Connection::Connection(QUrl server, QObject* parent)
+Connection::Connection(const QUrl& server, QObject* parent)
     : QObject(parent)
     , d(new Private(server))
 {
@@ -75,10 +75,11 @@ Connection::Connection()
 Connection::~Connection()
 {
     qCDebug(MAIN) << "deconstructing connection object for" << d->userId;
+    stopSync();
     delete d;
 }
 
-void Connection::resolveServer(QString domain)
+void Connection::resolveServer(const QString& domain)
 {
     // Find the Matrix server for the given domain.
     QScopedPointer<QDnsLookup, QScopedPointerDeleteLater> dns { new QDnsLookup() };
@@ -102,23 +103,21 @@ void Connection::resolveServer(QString domain)
     });
 }
 
-void Connection::connectToServer(QString user, QString password)
+void Connection::connectToServer(const QString& user, const QString& password)
 {
-    PasswordLogin* loginJob = new PasswordLogin(d->data, user, password);
+    auto loginJob = callApi<PasswordLogin>(user, password);
     connect( loginJob, &PasswordLogin::success, [=] () {
         connectWithToken(loginJob->id(), loginJob->token());
     });
     connect( loginJob, &PasswordLogin::failure, [=] () {
         emit loginError(loginJob->errorString());
     });
-    loginJob->start();
     d->username = user; // to be able to reconnect
     d->password = password;
 }
 
-void Connection::connectWithToken(QString userId, QString token)
+void Connection::connectWithToken(const QString& userId, const QString& token)
 {
-    d->isConnected = true;
     d->userId = userId;
     d->data->setToken(token);
     qCDebug(MAIN) << "Accessing" << d->data->baseUrl()
@@ -130,22 +129,14 @@ void Connection::connectWithToken(QString userId, QString token)
 
 void Connection::reconnect()
 {
-    PasswordLogin* loginJob = new PasswordLogin(d->data, d->username, d->password );
+    auto loginJob = callApi<PasswordLogin>(d->username, d->password);
     connect( loginJob, &PasswordLogin::success, [=] () {
         d->userId = loginJob->id();
         emit reconnected();
     });
     connect( loginJob, &PasswordLogin::failure, [=] () {
         emit loginError(loginJob->errorString());
-        d->isConnected = false;
     });
-    loginJob->start();
-}
-
-void Connection::disconnectFromServer()
-{
-    stopSync();
-    d->isConnected = false;
 }
 
 void Connection::logout()
@@ -162,7 +153,8 @@ void Connection::sync(int timeout)
     if (d->syncJob)
         return;
 
-    const QString filter = "{\"room\": { \"timeline\": { \"limit\": 100 } } }";
+    // Raw string: http://en.cppreference.com/w/cpp/language/string_literal
+    const QString filter { R"({"room": { "timeline": { "limit": 100 } } })" };
     auto job = d->syncJob =
             callApi<SyncJob>(d->data->lastEvent(), filter, timeout);
     connect( job, &SyncJob::success, [=] () {
@@ -194,20 +186,17 @@ void Connection::stopSync()
     }
 }
 
-void Connection::postMessage(Room* room, QString type, QString message)
+void Connection::postMessage(Room* room, const QString& type, const QString& message) const
 {
-    PostMessageJob* job = new PostMessageJob(d->data, room->id(), type, message);
-    job->start();
+    callApi<PostMessageJob>(room->id(), type, message);
 }
 
-PostReceiptJob* Connection::postReceipt(Room* room, Event* event)
+PostReceiptJob* Connection::postReceipt(Room* room, Event* event) const
 {
-    PostReceiptJob* job = new PostReceiptJob(d->data, room->id(), event->id());
-    job->start();
-    return job;
+    return callApi<PostReceiptJob>(room->id(), event->id());
 }
 
-JoinRoomJob* Connection::joinRoom(QString roomAlias)
+JoinRoomJob* Connection::joinRoom(const QString& roomAlias)
 {
     auto job = callApi<JoinRoomJob>(roomAlias);
     connect( job, &BaseJob::success, [=] () {
@@ -219,25 +208,21 @@ JoinRoomJob* Connection::joinRoom(QString roomAlias)
 
 void Connection::leaveRoom(Room* room)
 {
-    LeaveRoomJob* job = new LeaveRoomJob(d->data, room);
-    job->start();
+    callApi<LeaveRoomJob>(room);
 }
 
-RoomMessagesJob* Connection::getMessages(Room* room, QString from)
+RoomMessagesJob* Connection::getMessages(Room* room, const QString& from) const
 {
-    RoomMessagesJob* job = new RoomMessagesJob(d->data, room->id(), from);
-    job->start();
-    return job;
+    return callApi<RoomMessagesJob>(room->id(), from);
 }
 
-MediaThumbnailJob* Connection::getThumbnail(QUrl url, QSize requestedSize)
+MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, QSize requestedSize) const
 {
-    MediaThumbnailJob* job = new MediaThumbnailJob(d->data, url, requestedSize);
-    job->start();
-    return job;
+    return callApi<MediaThumbnailJob>(url, requestedSize);
 }
 
-MediaThumbnailJob* Connection::getThumbnail(QUrl url, int requestedWidth, int requestedHeight)
+MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, int requestedWidth,
+                                            int requestedHeight) const
 {
     return getThumbnail(url, QSize(requestedWidth, requestedHeight));
 }
@@ -247,7 +232,7 @@ QUrl Connection::homeserver() const
     return d->data->baseUrl();
 }
 
-User* Connection::user(QString userId)
+User* Connection::user(const QString& userId)
 {
     if( d->userMap.contains(userId) )
         return d->userMap.value(userId);
@@ -293,17 +278,12 @@ QHash< QString, Room* > Connection::roomMap() const
     return d->roomMap;
 }
 
-bool Connection::isConnected()
-{
-    return d->isConnected;
-}
-
-ConnectionData* Connection::connectionData()
+const ConnectionData* Connection::connectionData() const
 {
     return d->data;
 }
 
-Room* Connection::provideRoom(QString id)
+Room* Connection::provideRoom(const QString& id)
 {
     if (id.isEmpty())
     {
@@ -327,12 +307,12 @@ Room* Connection::provideRoom(QString id)
     return room;
 }
 
-User* Connection::createUser(QString userId)
+User* Connection::createUser(const QString& userId)
 {
     return new User(userId, this);
 }
 
-Room* Connection::createRoom(QString roomId)
+Room* Connection::createRoom(const QString& roomId)
 {
     return new Room(this, roomId);
 }
diff --git a/connection.h b/connection.h
index 849106a1..8fc4a6b3 100644
--- a/connection.h
+++ b/connection.h
@@ -39,34 +39,38 @@ namespace QMatrixClient
     class Connection: public QObject {
             Q_OBJECT
         public:
-            Connection(QUrl server, QObject* parent = nullptr);
+            explicit Connection(const QUrl& server, QObject* parent = nullptr);
             Connection();
             virtual ~Connection();
 
             QHash<QString, Room*> roomMap() const;
-            Q_INVOKABLE virtual bool isConnected();
 
-            Q_INVOKABLE virtual void resolveServer( QString domain );
-            Q_INVOKABLE virtual void connectToServer( QString user, QString password );
-            Q_INVOKABLE virtual void connectWithToken( QString userId, QString token );
+            Q_INVOKABLE virtual void resolveServer(const QString& domain);
+            Q_INVOKABLE virtual void connectToServer(const QString& user,
+                                                     const QString& password);
+            Q_INVOKABLE virtual void connectWithToken(const QString& userId,
+                                                      const QString& token);
             Q_INVOKABLE virtual void reconnect();
-            Q_INVOKABLE virtual void disconnectFromServer();
             Q_INVOKABLE virtual void logout();
 
-            Q_INVOKABLE virtual void sync(int timeout=-1);
-            Q_INVOKABLE virtual void stopSync();
+            Q_INVOKABLE void sync(int timeout = -1);
+            Q_INVOKABLE void stopSync();
             /** @deprecated Use callApi<PostMessageJob>() or Room::postMessage() instead */
-            Q_INVOKABLE virtual void postMessage( Room* room, QString type, QString message );
+            Q_INVOKABLE virtual void postMessage(Room* room, const QString& type,
+                                                 const QString& message) const;
             /** @deprecated Use callApi<PostReceiptJob>() or Room::postReceipt() instead */
-            Q_INVOKABLE virtual PostReceiptJob* postReceipt( Room* room, Event* event );
-            Q_INVOKABLE virtual JoinRoomJob* joinRoom( QString roomAlias );
+            Q_INVOKABLE virtual PostReceiptJob* postReceipt( Room* room, Event* event ) const;
+            Q_INVOKABLE virtual JoinRoomJob* joinRoom(const QString& roomAlias);
             Q_INVOKABLE virtual void leaveRoom( Room* room );
-            Q_INVOKABLE virtual RoomMessagesJob* getMessages( Room* room, QString from );
-            virtual MediaThumbnailJob* getThumbnail( QUrl url, QSize requestedSize );
-            MediaThumbnailJob* getThumbnail( QUrl url, int requestedWidth, int requestedHeight );
+            Q_INVOKABLE virtual RoomMessagesJob* getMessages(Room* room,
+                                                             const QString& from) const;
+            virtual MediaThumbnailJob* getThumbnail(const QUrl& url,
+                                                    QSize requestedSize) const;
+            MediaThumbnailJob* getThumbnail(const QUrl& url, int requestedWidth,
+                                            int requestedHeight) const;
 
             Q_INVOKABLE QUrl homeserver() const;
-            Q_INVOKABLE User* user(QString userId);
+            Q_INVOKABLE User* user(const QString& userId);
             Q_INVOKABLE User* user();
             Q_INVOKABLE QString userId() const;
             /** @deprecated Use accessToken() instead. */
@@ -76,7 +80,7 @@ namespace QMatrixClient
             Q_INVOKABLE int millisToReconnect() const;
 
             template <typename JobT, typename... JobArgTs>
-            JobT* callApi(JobArgTs... jobArgs)
+            JobT* callApi(JobArgTs... jobArgs) const
             {
                 auto job = new JobT(connectionData(), jobArgs...);
                 job->start();
@@ -103,7 +107,7 @@ namespace QMatrixClient
             /**
              * @brief Access the underlying ConnectionData class
              */
-            ConnectionData* connectionData();
+            const ConnectionData* connectionData() const;
 
             /**
              * @brief Find a (possibly new) Room object for the specified id
@@ -114,20 +118,20 @@ namespace QMatrixClient
              * @return a pointer to a Room object with the specified id; nullptr
              * if roomId is empty if createRoom() failed to create a Room object.
              */
-            Room* provideRoom(QString roomId);
+            Room* provideRoom(const QString& roomId);
 
             /**
              * makes it possible for derived classes to have its own User class
              */
-            virtual User* createUser(QString userId);
+            virtual User* createUser(const QString& userId);
 
             /**
              * makes it possible for derived classes to have its own Room class
              */
-            virtual Room* createRoom(QString roomId);
+            virtual Room* createRoom(const QString& roomId);
 
         private:
             class Private;
             Private* d;
     };
-}
+}  // namespace QMatrixClient
diff --git a/events/receiptevent.h b/events/receiptevent.h
index a7e1debf..40c0384f 100644
--- a/events/receiptevent.h
+++ b/events/receiptevent.h
@@ -20,8 +20,6 @@
 
 #include "event.h"
 
-#include <QtCore/QStringList>
-
 namespace QMatrixClient
 {
     class Receipt
diff --git a/events/roommemberevent.h b/events/roommemberevent.h
index f37cdc04..a33c2982 100644
--- a/events/roommemberevent.h
+++ b/events/roommemberevent.h
@@ -18,11 +18,10 @@
 
 #pragma once
 
-#include <QtCore/QJsonObject>
-#include <QtCore/QUrl>
-
 #include "event.h"
 
+#include <QtCore/QUrl>
+
 namespace QMatrixClient
 {
     enum class MembershipType {Invite, Join, Knock, Leave, Ban};
diff --git a/events/roommessageevent.h b/events/roommessageevent.h
index 67789ef7..5d5336aa 100644
--- a/events/roommessageevent.h
+++ b/events/roommessageevent.h
@@ -18,12 +18,12 @@
 
 #pragma once
 
+#include "event.h"
+
 #include <QtCore/QUrl>
 #include <QtCore/QMimeType>
 #include <QtCore/QSize>
 
-#include "event.h"
-
 namespace QMatrixClient
 {
     enum class MessageEventType
diff --git a/events/roomnameevent.cpp b/events/roomnameevent.cpp
index c5bcf011..c94cb2c3 100644
--- a/events/roomnameevent.cpp
+++ b/events/roomnameevent.cpp
@@ -22,8 +22,8 @@ using namespace QMatrixClient;
 

 class RoomNameEvent::Private

 {

-public:

-    QString name;

+    public:

+        QString name;

 };

 

 RoomNameEvent::RoomNameEvent() :

diff --git a/events/roomnameevent.h b/events/roomnameevent.h
index 0997ad9c..8748c4be 100644
--- a/events/roomnameevent.h
+++ b/events/roomnameevent.h
@@ -22,20 +22,18 @@
 

 namespace QMatrixClient

 {

+    class RoomNameEvent : public Event

+    {

+        public:

+            RoomNameEvent();

+            virtual ~RoomNameEvent();

 

-class RoomNameEvent : public Event

-{

-public:

-    RoomNameEvent();

-    virtual ~RoomNameEvent();

-

-    QString name() const;

-

-    static RoomNameEvent* fromJson(const QJsonObject& obj);

+            QString name() const;

 

-private:

-    class Private;

-    Private *d;

-};

+            static RoomNameEvent* fromJson(const QJsonObject& obj);

 

+        private:

+            class Private;

+            Private *d;

+    };

 }

diff --git a/events/roomtopicevent.h b/events/roomtopicevent.h
index d4347953..4b0a24b0 100644
--- a/events/roomtopicevent.h
+++ b/events/roomtopicevent.h
@@ -18,8 +18,6 @@
 
 #pragma once
 
-#include <QtCore/QJsonObject>
-
 #include "event.h"
 
 namespace QMatrixClient
diff --git a/events/typingevent.h b/events/typingevent.h
index 5a8b045c..da57a389 100644
--- a/events/typingevent.h
+++ b/events/typingevent.h
@@ -18,10 +18,10 @@
 
 #pragma once
 
-#include <QtCore/QStringList>
-
 #include "event.h"
 
+#include <QtCore/QStringList>
+
 namespace QMatrixClient
 {
     class TypingEvent: public Event
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 3e2ecd54..f5de75f4 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -24,7 +24,6 @@
 #include <QtNetwork/QNetworkAccessManager>
 #include <QtNetwork/QNetworkRequest>
 #include <QtNetwork/QNetworkReply>
-#include <QtNetwork/QSslError>
 #include <QtCore/QTimer>
 
 #include "../connectiondata.h"
@@ -44,16 +43,18 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater
 class BaseJob::Private
 {
     public:
-        Private(ConnectionData* c, HttpVerb v, QString endpoint,
-                const QUrlQuery& q, const Data& data, bool nt)
-            : connection(c), verb(v), apiEndpoint(endpoint), requestQuery(q)
-            , requestData(data), needsToken(nt)
-            , reply(nullptr), status(NoError)
+        // Using an idiom from clang-tidy:
+        // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html
+        Private(const ConnectionData* c, HttpVerb v,
+                QString endpoint, QUrlQuery q, Data data, bool nt)
+            : connection(c), verb(v), apiEndpoint(std::move(endpoint))
+            , requestQuery(std::move(q)), requestData(std::move(data))
+            , needsToken(nt), reply(nullptr), status(NoError)
         { }
         
         inline void sendRequest();
 
-        ConnectionData* connection;
+        const ConnectionData* connection;
 
         // Contents for the network request
         HttpVerb verb;
@@ -77,9 +78,9 @@ inline QDebug operator<<(QDebug dbg, const BaseJob* j)
     return dbg << "Job" << j->objectName();
 }
 
-BaseJob::BaseJob(ConnectionData* connection, HttpVerb verb, QString name,
-                 QString endpoint, BaseJob::Query query, BaseJob::Data data,
-                 bool needsToken)
+BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb,
+                 const QString& name, const QString& endpoint,
+                 const Query& query, const Data& data, bool needsToken)
     : d(new Private(connection, verb, endpoint, query, data, needsToken))
 {
     setObjectName(name);
@@ -96,12 +97,12 @@ BaseJob::~BaseJob()
     qCDebug(JOBS) << this << "destroyed";
 }
 
-ConnectionData* BaseJob::connection() const
+const ConnectionData* BaseJob::connection() const
 {
     return d->connection;
 }
 
-const QUrlQuery&BaseJob::query() const
+const QUrlQuery& BaseJob::query() const
 {
     return d->requestQuery;
 }
@@ -111,7 +112,7 @@ void BaseJob::setRequestQuery(const QUrlQuery& query)
     d->requestQuery = query;
 }
 
-const BaseJob::Data&BaseJob::requestData() const
+const BaseJob::Data& BaseJob::requestData() const
 {
     return d->requestData;
 }
diff --git a/jobs/basejob.h b/jobs/basejob.h
index ed35e3e8..460af0fc 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -100,7 +100,7 @@ namespace QMatrixClient
             {
                 public:
                     Status(StatusCode c) : code(c) { }
-                    Status(int c, QString m) : code(c), message(m) { }
+                    Status(int c, QString m) : code(c), message(std::move(m)) { }
 
                     bool good() const { return code < ErrorLevel; }
 
@@ -111,10 +111,10 @@ namespace QMatrixClient
             using duration_t = int; // milliseconds
 
         public:
-            BaseJob(ConnectionData* connection, HttpVerb verb, QString name,
-                    QString endpoint, Query query = Query(), Data data = Data(),
+            BaseJob(const ConnectionData* connection, HttpVerb verb,
+                    const QString& name, const QString& endpoint,
+                    const Query& query = {}, const Data& data = {},
                     bool needsToken = true);
-            virtual ~BaseJob();
 
             Status status() const;
             int error() const;
@@ -198,7 +198,7 @@ namespace QMatrixClient
             void failure(BaseJob*);
 
         protected:
-            ConnectionData* connection() const;
+            const ConnectionData* connection() const;
 
             const QUrlQuery& query() const;
             void setRequestQuery(const QUrlQuery& query);
@@ -241,6 +241,8 @@ namespace QMatrixClient
             void setStatus(Status s);
             void setStatus(int code, QString message);
 
+            // Job objects should only be deleted via QObject::deleteLater
+            virtual ~BaseJob();
         protected slots:
             void timeout();
             void sslErrors(const QList<QSslError>& errors);
diff --git a/jobs/checkauthmethods.cpp b/jobs/checkauthmethods.cpp
index 5c6a95d2..95b9a8f2 100644
--- a/jobs/checkauthmethods.cpp
+++ b/jobs/checkauthmethods.cpp
@@ -18,25 +18,18 @@
 
 #include "checkauthmethods.h"
 
-#include <QtNetwork/QNetworkAccessManager>
-#include <QtNetwork/QNetworkReply>
 #include <QtCore/QJsonDocument>
 #include <QtCore/QJsonObject>
-#include <QtCore/QJsonParseError>
-
-#include "../connectiondata.h"
 
 using namespace QMatrixClient;
 
 class CheckAuthMethods::Private
 {
     public:
-        Private() {}
-        
         QString session;
 };
 
-CheckAuthMethods::CheckAuthMethods(ConnectionData* connection)
+CheckAuthMethods::CheckAuthMethods(const ConnectionData* connection)
     : BaseJob(connection, HttpVerb::Get, "CheckAuthMethods",
               "_matrix/client/r0/login", Query(), Data(), false)
     , d(new Private)
diff --git a/jobs/checkauthmethods.h b/jobs/checkauthmethods.h
index f6eb978a..7d7dc40f 100644
--- a/jobs/checkauthmethods.h
+++ b/jobs/checkauthmethods.h
@@ -22,12 +22,10 @@
 
 namespace QMatrixClient
 {
-    class ConnectionData;
-
     class CheckAuthMethods : public BaseJob
     {
         public:
-            CheckAuthMethods(ConnectionData* connection);
+            CheckAuthMethods(const ConnectionData* connection);
             virtual ~CheckAuthMethods();
 
             QString session();
diff --git a/jobs/joinroomjob.cpp b/jobs/joinroomjob.cpp
index dce1f54e..6278c18b 100644
--- a/jobs/joinroomjob.cpp
+++ b/jobs/joinroomjob.cpp
@@ -19,10 +19,6 @@
 #include "joinroomjob.h"
 #include "util.h"
 
-#include <QtNetwork/QNetworkReply>
-
-#include "../connectiondata.h"
-
 using namespace QMatrixClient;
 
 class JoinRoomJob::Private
@@ -31,7 +27,7 @@ class JoinRoomJob::Private
         QString roomId;
 };
 
-JoinRoomJob::JoinRoomJob(ConnectionData* data, QString roomAlias)
+JoinRoomJob::JoinRoomJob(const ConnectionData* data, const QString& roomAlias)
     : BaseJob(data, HttpVerb::Post, "JoinRoomJob",
               QString("_matrix/client/r0/join/%1").arg(roomAlias))
     , d(new Private)
diff --git a/jobs/joinroomjob.h b/jobs/joinroomjob.h
index a69843ed..7cf90fd5 100644
--- a/jobs/joinroomjob.h
+++ b/jobs/joinroomjob.h
@@ -22,21 +22,19 @@
 
 namespace QMatrixClient
 {
-    class ConnectionData;
-
     class JoinRoomJob: public BaseJob
     {
         public:
-            JoinRoomJob(ConnectionData* data, QString roomAlias);
+            JoinRoomJob(const ConnectionData* data, const QString& roomAlias);
             virtual ~JoinRoomJob();
 
             QString roomId();
 
-            protected:
-                Status parseJson(const QJsonDocument& data) override;
+        protected:
+            Status parseJson(const QJsonDocument& data) override;
 
-            private:
-                class Private;
-                Private* d;
+        private:
+            class Private;
+            Private* d;
     };
-}
+}  // namespace QMatrixClient
diff --git a/jobs/leaveroomjob.cpp b/jobs/leaveroomjob.cpp
index 22a5d34b..5557b8e4 100644
--- a/jobs/leaveroomjob.cpp
+++ b/jobs/leaveroomjob.cpp
@@ -22,10 +22,7 @@
 
 using namespace QMatrixClient;
 
-LeaveRoomJob::LeaveRoomJob(ConnectionData* data, Room* room)
+LeaveRoomJob::LeaveRoomJob(const ConnectionData* data, Room* room)
     : BaseJob(data, HttpVerb::Post, "LeaveRoomJob",
               QString("_matrix/client/r0/rooms/%1/leave").arg(room->id()))
 { }
-
-LeaveRoomJob::~LeaveRoomJob()
-{ }
diff --git a/jobs/leaveroomjob.h b/jobs/leaveroomjob.h
index 4a62810f..03851f4c 100644
--- a/jobs/leaveroomjob.h
+++ b/jobs/leaveroomjob.h
@@ -22,13 +22,11 @@
 
 namespace QMatrixClient
 {
-    class ConnectionData;
     class Room;
 
     class LeaveRoomJob: public BaseJob
     {
         public:
-            LeaveRoomJob(ConnectionData* data, Room* room);
-            virtual ~LeaveRoomJob();
+            LeaveRoomJob(const ConnectionData* data, Room* room);
     };
 }
diff --git a/jobs/logoutjob.cpp b/jobs/logoutjob.cpp
index 9b9cacb6..84e88760 100644
--- a/jobs/logoutjob.cpp
+++ b/jobs/logoutjob.cpp
@@ -20,11 +20,7 @@
 
 using namespace QMatrixClient;
 
-LogoutJob::LogoutJob(ConnectionData* connection)
+LogoutJob::LogoutJob(const ConnectionData* connection)
     : BaseJob(connection, HttpVerb::Post, "LogoutJob", "/_matrix/client/r0/logout")
 {
 }
-
-LogoutJob::~LogoutJob()
-{
-}
diff --git a/jobs/logoutjob.h b/jobs/logoutjob.h
index 7d70a74c..780719e4 100644
--- a/jobs/logoutjob.h
+++ b/jobs/logoutjob.h
@@ -25,7 +25,6 @@ namespace QMatrixClient
     class LogoutJob: public BaseJob
     {
         public:
-            LogoutJob(ConnectionData* connection);
-            virtual ~LogoutJob();
+            explicit LogoutJob(const ConnectionData* connection);
     };
 }
diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp
index cfde902a..9bb731b9 100644
--- a/jobs/mediathumbnailjob.cpp
+++ b/jobs/mediathumbnailjob.cpp
@@ -29,7 +29,7 @@ class MediaThumbnailJob::Private
         QPixmap thumbnail;
 };
 
-MediaThumbnailJob::MediaThumbnailJob(ConnectionData* data, QUrl url, QSize requestedSize,
+MediaThumbnailJob::MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize,
                                      ThumbnailType thumbnailType)
     : BaseJob(data, HttpVerb::Get, "MediaThumbnailJob",
               QString("/_matrix/media/v1/thumbnail/%1%2").arg(url.host(), url.path()),
diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h
index cf1e9afb..307d0a99 100644
--- a/jobs/mediathumbnailjob.h
+++ b/jobs/mediathumbnailjob.h
@@ -29,7 +29,7 @@ namespace QMatrixClient
     class MediaThumbnailJob: public BaseJob
     {
         public:
-            MediaThumbnailJob(ConnectionData* data, QUrl url, QSize requestedSize,
+            MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize,
                               ThumbnailType thumbnailType=ThumbnailType::Scale);
             virtual ~MediaThumbnailJob();
 
diff --git a/jobs/passwordlogin.cpp b/jobs/passwordlogin.cpp
index 2f34e86a..081e19bc 100644
--- a/jobs/passwordlogin.cpp
+++ b/jobs/passwordlogin.cpp
@@ -18,10 +18,6 @@
 
 #include "passwordlogin.h"
 
-#include <QtNetwork/QNetworkReply>
-
-#include "../connectiondata.h"
-
 using namespace QMatrixClient;
 
 class PasswordLogin::Private
@@ -32,7 +28,7 @@ class PasswordLogin::Private
         QString returned_token;
 };
 
-PasswordLogin::PasswordLogin(ConnectionData* connection, QString user, QString password)
+PasswordLogin::PasswordLogin(const ConnectionData* connection, QString user, QString password)
     : BaseJob(connection, HttpVerb::Post, "PasswordLogin"
             , "_matrix/client/r0/login"
             , Query()
diff --git a/jobs/passwordlogin.h b/jobs/passwordlogin.h
index c1291389..713a1821 100644
--- a/jobs/passwordlogin.h
+++ b/jobs/passwordlogin.h
@@ -22,12 +22,11 @@
 
 namespace QMatrixClient
 {
-    class ConnectionData;
-
     class PasswordLogin : public BaseJob
     {
         public:
-            PasswordLogin(ConnectionData* connection, QString user, QString password);
+            PasswordLogin(const ConnectionData* connection,
+                          QString user, QString password);
             virtual ~PasswordLogin();
 
             QString token();
diff --git a/jobs/postmessagejob.cpp b/jobs/postmessagejob.cpp
index 9a102325..df30614c 100644
--- a/jobs/postmessagejob.cpp
+++ b/jobs/postmessagejob.cpp
@@ -17,33 +17,29 @@
  */
 
 #include "postmessagejob.h"
-#include "../connectiondata.h"
 #include "util.h"
 
-#include <QtNetwork/QNetworkReply>
-
 using namespace QMatrixClient;
 
 class PostMessageJob::Private
 {
     public:
-        Private() {}
-
         QString eventId; // unused yet
 };
 
-PostMessageJob::PostMessageJob(ConnectionData* connection, const QString& roomId,
-                               const QString& type, const QString& plainText)
+PostMessageJob::PostMessageJob(const ConnectionData* connection,
+                               const QString& roomId, const QString& type,
+                               const QString& plainText)
     : BaseJob(connection, HttpVerb::Post, "PostMessageJob",
-              QString("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId),
+              QStringLiteral("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId),
               Query(),
               Data({ { "msgtype", type }, { "body", plainText } }) )
     , d(new Private)
 { }
 
-PostMessageJob::PostMessageJob(ConnectionData* connection, const QString& roomId,
-                               const QString& type, const QString& plainText,
-                               const QString& richText)
+PostMessageJob::PostMessageJob(const ConnectionData* connection,
+                               const QString& roomId, const QString& type,
+                               const QString& plainText, const QString& richText)
     : BaseJob(connection, HttpVerb::Post, "PostMessageJob",
               QStringLiteral("_matrix/client/r0/rooms/%1/send/m.room.message").arg(roomId),
               Query(),
diff --git a/jobs/postmessagejob.h b/jobs/postmessagejob.h
index 14de52f0..f4ae809b 100644
--- a/jobs/postmessagejob.h
+++ b/jobs/postmessagejob.h
@@ -26,10 +26,10 @@ namespace QMatrixClient
     {
         public:
             /** Constructs a plain text message job */
-            PostMessageJob(ConnectionData* connection, const QString& roomId,
+            PostMessageJob(const ConnectionData* connection, const QString& roomId,
                            const QString& type, const QString& plainText);
             /** Constructs a rich text message job */
-            PostMessageJob(ConnectionData* connection, const QString& roomId,
+            PostMessageJob(const ConnectionData* connection, const QString& roomId,
                            const QString& type, const QString& plainText,
                            const QString& richText);
             virtual ~PostMessageJob();
@@ -43,4 +43,4 @@ namespace QMatrixClient
             class Private;
             Private* d;
     };
-}
+}  // namespace QMatrixClient
diff --git a/jobs/postreceiptjob.cpp b/jobs/postreceiptjob.cpp
index ee750dbf..00926de6 100644
--- a/jobs/postreceiptjob.cpp
+++ b/jobs/postreceiptjob.cpp
@@ -17,17 +17,11 @@
  */
 
 #include "postreceiptjob.h"
-#include "../room.h"
-#include "../connectiondata.h"
-
-#include <QtNetwork/QNetworkReply>
 
 using namespace QMatrixClient;
 
-PostReceiptJob::PostReceiptJob(ConnectionData* connection, QString roomId, QString eventId)
+PostReceiptJob::PostReceiptJob(const ConnectionData* connection,
+                               const QString& roomId, const QString& eventId)
     : BaseJob(connection, HttpVerb::Post, "PostReceiptJob",
               QString("/_matrix/client/r0/rooms/%1/receipt/m.read/%2").arg(roomId, eventId))
 { }
-
-PostReceiptJob::~PostReceiptJob()
-{ }
diff --git a/jobs/postreceiptjob.h b/jobs/postreceiptjob.h
index c0002dc0..1c84f411 100644
--- a/jobs/postreceiptjob.h
+++ b/jobs/postreceiptjob.h
@@ -25,7 +25,7 @@ namespace QMatrixClient
     class PostReceiptJob: public BaseJob
     {
         public:
-            PostReceiptJob(ConnectionData* connection, QString roomId, QString eventId);
-            virtual ~PostReceiptJob();
+            PostReceiptJob(const ConnectionData* connection, const QString& roomId,
+                           const QString& eventId);
     };
 }
diff --git a/jobs/roommessagesjob.cpp b/jobs/roommessagesjob.cpp
index 5779d695..a48403c8 100644
--- a/jobs/roommessagesjob.cpp
+++ b/jobs/roommessagesjob.cpp
@@ -26,14 +26,12 @@ using namespace QMatrixClient;
 class RoomMessagesJob::Private
 {
     public:
-        Private() {}
-
         Owning<Events> events;
         QString end;
 };
 
-RoomMessagesJob::RoomMessagesJob(ConnectionData* data, QString roomId,
-                                 QString from, int limit, FetchDirection dir)
+RoomMessagesJob::RoomMessagesJob(const ConnectionData* data, const QString& roomId,
+                                 const QString& from, int limit, FetchDirection dir)
     : BaseJob(data, HttpVerb::Get, "RoomMessagesJob",
               QString("/_matrix/client/r0/rooms/%1/messages").arg(roomId),
               Query(
diff --git a/jobs/roommessagesjob.h b/jobs/roommessagesjob.h
index af7d65df..2d15d9d4 100644
--- a/jobs/roommessagesjob.h
+++ b/jobs/roommessagesjob.h
@@ -29,8 +29,8 @@ namespace QMatrixClient
     class RoomMessagesJob: public BaseJob
     {
         public:
-            RoomMessagesJob(ConnectionData* data, QString roomId,
-                            QString from, int limit = 10,
+            RoomMessagesJob(const ConnectionData* data, const QString& roomId,
+                            const QString& from, int limit = 10,
                             FetchDirection dir = FetchDirection::Backward);
             virtual ~RoomMessagesJob();
 
@@ -44,4 +44,4 @@ namespace QMatrixClient
             class Private;
             Private* d;
     };
-}
+}  // namespace QMatrixClient
diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp
index 8db1e2ca..17c637b0 100644
--- a/jobs/syncjob.cpp
+++ b/jobs/syncjob.cpp
@@ -32,8 +32,8 @@ class SyncJob::Private
 
 static size_t jobId = 0;
 
-SyncJob::SyncJob(ConnectionData* connection,
-                 QString since, QString filter, int timeout, QString presence)
+SyncJob::SyncJob(const ConnectionData* connection, const QString& since,
+                 const QString& filter, int timeout, const QString& presence)
     : BaseJob(connection, HttpVerb::Get, QString("SyncJob-%1").arg(++jobId),
               "_matrix/client/r0/sync")
     , d(new Private)
@@ -84,11 +84,10 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
     for (auto roomState: roomStates)
     {
         const QJsonObject rs = rooms.value(roomState.jsonKey).toObject();
-        d->roomData.reserve(rs.size());
+        // We have a Qt container on the right and an STL one on the left
+        d->roomData.reserve(static_cast<size_t>(rs.size()));
         for( auto rkey: rs.keys() )
-        {
-            d->roomData.push_back({rkey, roomState.enumVal, rs[rkey].toObject()});
-        }
+            d->roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject());
     }
 
     return Success;
@@ -99,7 +98,8 @@ void SyncRoomData::EventList::fromJson(const QJsonObject& roomContents)
     assign(eventsFromJson(roomContents[jsonKey].toObject()["events"].toArray()));
 }
 
-SyncRoomData::SyncRoomData(QString roomId_, JoinState joinState_, const QJsonObject& room_)
+SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
+                           const QJsonObject& room_)
     : roomId(roomId_)
     , joinState(joinState_)
     , state("state")
diff --git a/jobs/syncjob.h b/jobs/syncjob.h
index b41c09d4..21d3cfca 100644
--- a/jobs/syncjob.h
+++ b/jobs/syncjob.h
@@ -34,7 +34,7 @@ namespace QMatrixClient
             private:
                 QString jsonKey;
             public:
-                explicit EventList(QString k) : jsonKey(k) { }
+                explicit EventList(QString k) : jsonKey(std::move(k)) { }
                 void fromJson(const QJsonObject& roomContents);
         };
 
@@ -51,11 +51,10 @@ namespace QMatrixClient
         int highlightCount;
         int notificationCount;
 
-        SyncRoomData(QString roomId_ = QString(),
-                     JoinState joinState_ = JoinState::Join,
-                     const QJsonObject& room_ = QJsonObject());
+        SyncRoomData(const QString& roomId, JoinState joinState_,
+                     const QJsonObject& room_);
     };
-}
+}  // namespace QMatrixClient
 Q_DECLARE_TYPEINFO(QMatrixClient::SyncRoomData, Q_MOVABLE_TYPE);
 
 namespace QMatrixClient
@@ -63,12 +62,12 @@ namespace QMatrixClient
     // QVector cannot work with non-copiable objects, std::vector can.
     using SyncData = std::vector<SyncRoomData>;
 
-    class ConnectionData;
     class SyncJob: public BaseJob
     {
         public:
-            SyncJob(ConnectionData* connection, QString since = {}, QString filter = {},
-                    int timeout = -1, QString presence = {});
+            explicit SyncJob(const ConnectionData* connection, const QString& since = {},
+                             const QString& filter = {},
+                             int timeout = -1, const QString& presence = {});
             virtual ~SyncJob();
 
             SyncData& roomData();
@@ -81,4 +80,4 @@ namespace QMatrixClient
             class Private;
             Private* d;
     };
-}
+}  // namespace QMatrixClient
diff --git a/room.cpp b/room.cpp
index f13fae19..5159cafd 100644
--- a/room.cpp
+++ b/room.cpp
@@ -115,7 +115,7 @@ class Room::Private
         /**
          * Removes events from the passed container that are already in the timeline
          */
-        void dropDuplicateEvents(Events& events) const;
+        void dropDuplicateEvents(Events* events) const;
 
         void setLastReadEvent(User* u, QString eventId);
         rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker);
@@ -510,7 +510,7 @@ QString Room::roomMembername(User *u) const
     return username % " (" % u->id() % ")";
 }
 
-QString Room::roomMembername(QString userId) const
+QString Room::roomMembername(const QString& userId) const
 {
     return roomMembername(connection()->user(userId));
 }
@@ -577,15 +577,15 @@ void Room::Private::getPreviousContent(int limit)
     }
 }
 
-void Room::Private::dropDuplicateEvents(Events& events) const
+void Room::Private::dropDuplicateEvents(Events* events) const
 {
     // Collect all duplicate events at the end of the container
     auto dupsBegin =
-            std::stable_partition(events.begin(), events.end(),
+            std::stable_partition(events->begin(), events->end(),
                                   [&] (Event* e) { return !eventsIndex.contains(e->id()); });
     // Dispose of those dups
-    std::for_each(dupsBegin, events.end(), [] (Event* e) { delete e; });
-    events.erase(dupsBegin, events.end());
+    std::for_each(dupsBegin, events->end(), [] (Event* e) { delete e; });
+    events->erase(dupsBegin, events->end());
 }
 
 Connection* Room::connection() const
@@ -595,7 +595,7 @@ Connection* Room::connection() const
 
 void Room::addNewMessageEvents(Events events)
 {
-    d->dropDuplicateEvents(events);
+    d->dropDuplicateEvents(&events);
     if (events.empty())
         return;
     emit aboutToAddNewMessages(events);
@@ -640,7 +640,7 @@ void Room::doAddNewMessageEvents(const Events& events)
 
 void Room::addHistoricalMessageEvents(Events events)
 {
-    d->dropDuplicateEvents(events);
+    d->dropDuplicateEvents(&events);
     if (events.empty())
         return;
     emit aboutToAddHistoricalMessages(events);
diff --git a/room.h b/room.h
index 315e4016..8b810ae4 100644
--- a/room.h
+++ b/room.h
@@ -63,13 +63,13 @@ namespace QMatrixClient
     class Room: public QObject
     {
             Q_OBJECT
-            Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved)
             Q_PROPERTY(QString id READ id CONSTANT)
             Q_PROPERTY(QString name READ name NOTIFY namesChanged)
             Q_PROPERTY(QStringList aliases READ aliases NOTIFY namesChanged)
             Q_PROPERTY(QString canonicalAlias READ canonicalAlias NOTIFY namesChanged)
             Q_PROPERTY(QString displayName READ displayName NOTIFY namesChanged)
             Q_PROPERTY(QString topic READ topic NOTIFY topicChanged)
+            Q_PROPERTY(QString readMarkerEventId READ readMarkerEventId WRITE markMessagesAsRead NOTIFY readMarkerMoved)
         public:
             using Timeline = std::deque<TimelineItem>;
             using rev_iter_t = Timeline::const_reverse_iterator;
@@ -98,7 +98,7 @@ namespace QMatrixClient
              * @brief Produces a disambiguated name for a user with this id in
              * the context of the room
              */
-            Q_INVOKABLE QString roomMembername(QString userId) const;
+            Q_INVOKABLE QString roomMembername(const QString& userId) const;
 
             Q_INVOKABLE void updateData(SyncRoomData& data );
             Q_INVOKABLE void setJoinState( JoinState state );
diff --git a/user.cpp b/user.cpp
index c5dac921..17714bee 100644
--- a/user.cpp
+++ b/user.cpp
@@ -34,6 +34,12 @@ using namespace QMatrixClient;
 class User::Private
 {
     public:
+        Private(QString userId, Connection* connection)
+            : q(nullptr), userId(std::move(userId)), connection(connection)
+            , defaultIcon(QIcon::fromTheme(QStringLiteral("user-available")))
+            , avatarValid(false) , avatarOngoingRequest(false)
+        { }
+
         User* q;
         QString userId;
         QString name;
@@ -51,14 +57,9 @@ class User::Private
 };
 
 User::User(QString userId, Connection* connection)
-    : QObject(connection), d(new Private)
+    : QObject(connection), d(new Private(userId, connection))
 {
-    d->connection = connection;
-    d->userId = userId;
-    d->avatarValid = false;
-    d->avatarOngoingRequest = false;
-    d->q = this;
-    d->defaultIcon = QIcon::fromTheme(QStringLiteral("user-available"));
+    d->q = this; // Initialization finished
 }
 
 User::~User()