From 5038ae0a0099c2a5c6ffdd08734b597d92edac70 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 7 May 2017 20:02:34 +0900 Subject: 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. --- connection.cpp | 80 ++++++++++++++++++++++------------------------------------ 1 file changed, 30 insertions(+), 50 deletions(-) (limited to 'connection.cpp') 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 roomMap; QHash 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 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(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(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(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(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(room->id(), event->id()); } -JoinRoomJob* Connection::joinRoom(QString roomAlias) +JoinRoomJob* Connection::joinRoom(const QString& roomAlias) { auto job = callApi(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(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(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(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); } -- cgit v1.2.3