From 46aa93985d3836863a5ca1e66fbe503f49a6f611 Mon Sep 17 00:00:00 2001 From: Phil Hoffmann Date: Mon, 6 Nov 2023 13:40:56 +0100 Subject: [PATCH] Fixed even more code smells --- app/ui/FileDialog/filedialogbackend.cpp | 9 ++++--- src/addons/addonrepositorymanager.cpp | 7 +++--- src/filesystem/fileaccesslocal.cpp | 4 +--- src/filesystem/fileaccesslocal.h | 2 +- src/filesystem/fileaccessnextcloud.h | 2 +- src/services/nextcloud/nextcloud.cpp | 2 +- src/services/rest/restserviceconnector.cpp | 4 ++-- src/services/rest/restserviceconnectorlocal.h | 2 +- src/services/service.cpp | 6 ----- src/services/service.h | 3 --- src/services/spotify/api/albumapi.cpp | 8 +++---- src/services/spotify/api/playerapi.cpp | 7 +++--- src/tools/audio/audiotool.cpp | 5 ++-- src/tools/audio/audiotool.h | 2 +- src/tools/audio/editor/audioeditor.cpp | 4 ++-- src/tools/audio/editor/audioeditor.h | 2 +- .../audio/editor/audioeditorfilebrowser.cpp | 2 +- src/tools/audio/project/audioscenario.cpp | 3 +-- src/tools/characters/character.h | 2 +- .../viewers/characterimageviewer.cpp | 4 ++-- .../characters/viewers/characterimageviewer.h | 2 +- src/tools/maps/map.cpp | 24 +++++++++---------- src/tools/maps/map.h | 4 ++-- src/tools/notes/markdownhighlighter.cpp | 4 ++-- src/tools/notes/notessaveload.cpp | 2 +- src/tools/notes/notestool.cpp | 2 +- src/tools/notes/notestool.h | 2 +- src/tools/shop/itemeditor.cpp | 2 +- src/tools/shop/shoptool.cpp | 2 +- 29 files changed, 55 insertions(+), 69 deletions(-) diff --git a/app/ui/FileDialog/filedialogbackend.cpp b/app/ui/FileDialog/filedialogbackend.cpp index 098d3881..1c485182 100644 --- a/app/ui/FileDialog/filedialogbackend.cpp +++ b/app/ui/FileDialog/filedialogbackend.cpp @@ -125,11 +125,10 @@ void FileDialogBackend::updateFileList() isLoading(true); m_currentFuture = File::listAsync(currentDir(), !folderMode(), true); - m_currentFuture.then([this](FileListResult &&result) { onFileListReceived(std::move(result)); }) - .onCanceled([this]() { - qCDebug(gmFileDialog()) << "file list update was cancelled."; - isLoading(false); - }); + m_currentFuture.then([this](const FileListResult &result) { onFileListReceived(result); }).onCanceled([this]() { + qCDebug(gmFileDialog()) << "file list update was cancelled."; + isLoading(false); + }); } void FileDialogBackend::clearFileList() diff --git a/src/addons/addonrepositorymanager.cpp b/src/addons/addonrepositorymanager.cpp index 7083c49f..470d5a79 100644 --- a/src/addons/addonrepositorymanager.cpp +++ b/src/addons/addonrepositorymanager.cpp @@ -214,10 +214,9 @@ auto AddonRepositoryManager::parseRepositoryData(const QByteArray &data) -> std: auto release = getNewestCompatibleRelease(entry["releases"_L1].toArray()); if (release.isEmpty()) continue; - result.emplace_back(AddonReleaseInfo(entry["id"_L1].toString(), entry["name"_L1].toString(), - entry["name_short"_L1].toString(), release["version"_L1].toString(), - entry["author"_L1].toString(), entry["description"_L1].toString(), - release["download"_L1].toString())); + result.emplace_back(entry["id"_L1].toString(), entry["name"_L1].toString(), entry["name_short"_L1].toString(), + release["version"_L1].toString(), entry["author"_L1].toString(), + entry["description"_L1].toString(), release["download"_L1].toString()); } return result; diff --git a/src/filesystem/fileaccesslocal.cpp b/src/filesystem/fileaccesslocal.cpp index d35c1d56..0fd77a1b 100755 --- a/src/filesystem/fileaccesslocal.cpp +++ b/src/filesystem/fileaccesslocal.cpp @@ -150,9 +150,7 @@ auto FileAccessLocal::move(const QString &oldPath, const QString &newPath) -> Fi } } - QFile f(oldPath); - - if (!f.rename(newPath)) + if (QFile f(oldPath); !f.rename(newPath)) { auto errorMessage = u"Could not move %1 to %2: %3 %4"_s.arg(oldPath, newPath, QString::number(f.error()), f.errorString()); diff --git a/src/filesystem/fileaccesslocal.h b/src/filesystem/fileaccesslocal.h index eaf74cf2..2dcd36e7 100755 --- a/src/filesystem/fileaccesslocal.h +++ b/src/filesystem/fileaccesslocal.h @@ -11,7 +11,7 @@ class FileAccessLocal : public FileAccess { public: FileAccessLocal() = default; - virtual ~FileAccessLocal() = default; + ~FileAccessLocal() override = default; Q_DISABLE_COPY_MOVE(FileAccessLocal) auto getDataAsync(const QString &path, bool allowCache) -> QFuture override; diff --git a/src/filesystem/fileaccessnextcloud.h b/src/filesystem/fileaccessnextcloud.h index fe2e1bd4..37bf28de 100644 --- a/src/filesystem/fileaccessnextcloud.h +++ b/src/filesystem/fileaccessnextcloud.h @@ -13,7 +13,7 @@ class FileAccessNextcloud : public FileAccess { public: explicit FileAccessNextcloud(Services::NextCloud &nextcloud); - virtual ~FileAccessNextcloud() = default; + ~FileAccessNextcloud() override = default; Q_DISABLE_COPY_MOVE(FileAccessNextcloud) auto getDataAsync(const QString &path, bool allowCache) -> QFuture override; diff --git a/src/services/nextcloud/nextcloud.cpp b/src/services/nextcloud/nextcloud.cpp index 5366a7ed..166d9305 100644 --- a/src/services/nextcloud/nextcloud.cpp +++ b/src/services/nextcloud/nextcloud.cpp @@ -138,7 +138,7 @@ void NextCloud::disconnectService() SettingsManager::setPassword(loginName(), ""_L1, serviceName()); - disconnect(); + connected(false); } /** diff --git a/src/services/rest/restserviceconnector.cpp b/src/services/rest/restserviceconnector.cpp index 923b2f6a..632954d0 100644 --- a/src/services/rest/restserviceconnector.cpp +++ b/src/services/rest/restserviceconnector.cpp @@ -143,7 +143,7 @@ auto RESTServiceConnector::enqueueRequest(RestRequest &&request, QPromise O2Requestor *; void sendRequest(RestRequest &&container, QPromise &&promise) override; - [[nodiscard]] virtual auto getAccessToken() -> QString override; + [[nodiscard]] auto getAccessToken() -> QString override; void refreshAccessToken(bool /*updateAuthentication*/ = false) override; }; diff --git a/src/services/service.cpp b/src/services/service.cpp index 396402d2..d50f2fab 100644 --- a/src/services/service.cpp +++ b/src/services/service.cpp @@ -13,12 +13,6 @@ Service::Service(const QString &name, QObject *parent) connect(this, &Service::connectedChanged, this, &Service::onConnectedChanged); } -void Service::disconnect() -{ - SettingsManager::instance()->set(u"connected"_s, false, a_serviceName); - connected(false); -} - void Service::updateStatus(Status::Type type, const QString &message) { a_status->type(type); diff --git a/src/services/service.h b/src/services/service.h index cffe1d9f..c244cd95 100644 --- a/src/services/service.h +++ b/src/services/service.h @@ -24,9 +24,6 @@ public slots: virtual void connectService() = 0; virtual void disconnectService() = 0; -protected: - void disconnect(); - protected slots: void updateStatus(Status::Type type, const QString &message); diff --git a/src/services/spotify/api/albumapi.cpp b/src/services/spotify/api/albumapi.cpp index d009de60..502527a9 100644 --- a/src/services/spotify/api/albumapi.cpp +++ b/src/services/spotify/api/albumapi.cpp @@ -30,7 +30,7 @@ auto AlbumAPI::getAlbum(const QString &id) -> QFuture query.addQueryItem(u"market"_s, u"from_token"_s); url.setQuery(query); - const auto callback = [this](RestReply &&reply) { + const auto callback = [this](const RestReply &reply) { if (reply.hasError()) { qCWarning(gmSpotifyAlbums()) << "getAlbum():" << reply.errorText(); @@ -65,7 +65,7 @@ auto AlbumAPI::getAlbumTracks(const QString &id) -> QFuture query.addQueryItem(u"market"_s, u"from_token"_s); url.setQuery(query); - auto callback = [this](RestReply &&reply) -> QFuture { + auto callback = [this](const RestReply &reply) -> QFuture { if (reply.hasError()) { qCWarning(gmSpotifyAlbums()) << reply.errorText(); @@ -89,7 +89,7 @@ auto AlbumAPI::getAlbumTracks(SpotifyAlbum &&album) -> QFuture const QUrl url(album.tracks.next); - const auto callback = [this, album](RestReply &&reply) mutable { + const auto callback = [this, album](const RestReply &reply) mutable { if (reply.hasError()) { qCWarning(gmSpotifyAlbums()) << reply.errorText(); @@ -111,7 +111,7 @@ auto AlbumAPI::getAlbumTracks(SpotifyTrackList &&tracklist) -> QFuture auto PlayerAPI::play(const QString &uri) const -> QFuture { - const auto type = SpotifyUtils::getUriType(uri); - - if (type == SpotifyUtils::SpotifyType::Track || type == SpotifyUtils::SpotifyType::Episode) + if (const auto type = SpotifyUtils::getUriType(uri); + type == SpotifyUtils::SpotifyType::Track || type == SpotifyUtils::SpotifyType::Episode) { return play(makePlayBody(u""_s, {uri}, -1, -1)); } @@ -180,7 +179,7 @@ auto PlayerAPI::getState(const QStringList &additionalTypes, const QString &mark url.setQuery(query); - const auto callback = [](RestReply &&reply) -> QFuture { + const auto callback = [](const RestReply &reply) -> QFuture { if (reply.hasError()) { qCWarning(gmSpotifyPlayer()) << reply.errorText(); diff --git a/src/tools/audio/audiotool.cpp b/src/tools/audio/audiotool.cpp index 5bf130e8..bf095af6 100644 --- a/src/tools/audio/audiotool.cpp +++ b/src/tools/audio/audiotool.cpp @@ -47,7 +47,8 @@ AudioTool::AudioTool(QQmlEngine *engine, QObject *parent) }); connect(&mprisManager, &MprisManager::next, this, [this]() { next(); }); connect(&mprisManager, &MprisManager::previous, this, [this]() { again(); }); - connect(&mprisManager, &MprisManager::changeVolume, this, [this](double volume) { setMusicVolume(volume); }); + connect(&mprisManager, &MprisManager::changeVolume, this, + [this](double volume) { setMusicVolume(static_cast(volume)); }); #endif } @@ -300,7 +301,7 @@ auto AudioTool::playlistQml() -> QQmlListProperty * @brief Get index of current song in playlist * @return Index as integer */ -auto AudioTool::index() const -> int +auto AudioTool::index() const -> qsizetype { if (m_musicElementType == AudioElement::Type::Music) { diff --git a/src/tools/audio/audiotool.h b/src/tools/audio/audiotool.h index c18bfd1a..caa66ebf 100644 --- a/src/tools/audio/audiotool.h +++ b/src/tools/audio/audiotool.h @@ -89,7 +89,7 @@ class AudioTool : public AbstractTool { return metaDataReader.metaData(); } - [[nodiscard]] auto index() const -> int; + [[nodiscard]] auto index() const -> qsizetype; [[nodiscard]] auto playlistQml() -> QQmlListProperty; public slots: diff --git a/src/tools/audio/editor/audioeditor.cpp b/src/tools/audio/editor/audioeditor.cpp index 046a5a08..148c8ea9 100644 --- a/src/tools/audio/editor/audioeditor.cpp +++ b/src/tools/audio/editor/audioeditor.cpp @@ -13,7 +13,7 @@ using namespace Services; Q_LOGGING_CATEGORY(gmAudioEditor, "gm.audio.editor") -AudioEditor::AudioEditor(QQmlEngine *engine, QObject *parent) +AudioEditor::AudioEditor(const QQmlEngine *engine, QObject *parent) : AbstractTool(parent), m_addonElementManager(this), m_audioExporter(this), m_fileBrowser(this), m_unsplashParser(engine, this), m_fileModel(this) { @@ -99,7 +99,7 @@ auto AudioEditor::projectIndex() const -> int { if (!m_currentProject || a_projects.isEmpty()) return -1; - return a_projects.indexOf(m_currentProject); + return static_cast(a_projects.indexOf(m_currentProject)); } /** diff --git a/src/tools/audio/editor/audioeditor.h b/src/tools/audio/editor/audioeditor.h index 1200ac98..cd830dc7 100644 --- a/src/tools/audio/editor/audioeditor.h +++ b/src/tools/audio/editor/audioeditor.h @@ -32,7 +32,7 @@ class AudioEditor : public AbstractTool AUTO_PROPERTY_VAL2(bool, isSaved, true) public: - explicit AudioEditor(QQmlEngine *engine, QObject *parent = nullptr); + explicit AudioEditor(const QQmlEngine *engine, QObject *parent = nullptr); [[nodiscard]] auto exporter() -> AudioExporter * { diff --git a/src/tools/audio/editor/audioeditorfilebrowser.cpp b/src/tools/audio/editor/audioeditorfilebrowser.cpp index c08d021a..5498cb55 100644 --- a/src/tools/audio/editor/audioeditorfilebrowser.cpp +++ b/src/tools/audio/editor/audioeditorfilebrowser.cpp @@ -62,7 +62,7 @@ void AudioEditorFileBrowser::removeElement(const QStringList &path) { foreach (auto *element, m_fileModel.elements()) { - auto *file = qobject_cast(element); + const auto *file = qobject_cast(element); if ((file->path() == path) || FileUtils::dirFromFolders(file->path()).startsWith(FileUtils::dirFromFolders(path))) diff --git a/src/tools/audio/project/audioscenario.cpp b/src/tools/audio/project/audioscenario.cpp index 42c8bf4f..7a2b0926 100644 --- a/src/tools/audio/project/audioscenario.cpp +++ b/src/tools/audio/project/audioscenario.cpp @@ -365,9 +365,8 @@ auto AudioScenario::moveScenario(AudioScenario *scenario, int steps) -> bool } const auto from = a_scenarios.indexOf(scenario); - const auto to = from + steps; - if (Utils::isInBounds(a_scenarios, to)) + if (const auto to = from + steps; Utils::isInBounds(a_scenarios, to)) { a_scenarios.move(from, to); emit scenariosChanged(); diff --git a/src/tools/characters/character.h b/src/tools/characters/character.h index 50421326..9b0f61f9 100644 --- a/src/tools/characters/character.h +++ b/src/tools/characters/character.h @@ -49,7 +49,7 @@ class Character : public QObject signals: void fileListLoaded(const QList &files); - void fileDataLoaded(int index, const QByteArray &data); + void fileDataLoaded(qsizetype index, const QByteArray &data); private: QList m_files; diff --git a/src/tools/characters/viewers/characterimageviewer.cpp b/src/tools/characters/viewers/characterimageviewer.cpp index 85879720..e0a7ff1d 100644 --- a/src/tools/characters/viewers/characterimageviewer.cpp +++ b/src/tools/characters/viewers/characterimageviewer.cpp @@ -49,7 +49,7 @@ void CharacterImageViewer::onCharacterFileListLoaded(const QList(index), data); break; default: diff --git a/src/tools/characters/viewers/characterimageviewer.h b/src/tools/characters/viewers/characterimageviewer.h index d98713eb..49c320c0 100644 --- a/src/tools/characters/viewers/characterimageviewer.h +++ b/src/tools/characters/viewers/characterimageviewer.h @@ -43,5 +43,5 @@ class CharacterImageViewer : public CharacterViewer private slots: void onCharacterFileListLoaded(const QList &files); - void onCharacterFileDataLoaded(int index, const QByteArray &data); + void onCharacterFileDataLoaded(qsizetype index, const QByteArray &data); }; diff --git a/src/tools/maps/map.cpp b/src/tools/maps/map.cpp index 83349903..d13ed07c 100644 --- a/src/tools/maps/map.cpp +++ b/src/tools/maps/map.cpp @@ -24,7 +24,7 @@ Map::Map(QObject *parent) : QObject(parent), m_markers(this) Map::Map(const QString &name, const QString &path, QObject *parent) : QObject(parent), a_name(name), a_path(path), m_markers(this) { - Files::File::getDataAsync(path).then([this](Files::FileDataResult &&result) { + Files::File::getDataAsync(path).then([this](const Files::FileDataResult &result) { if (!result.success()) return; QPixmap pixmap; @@ -52,15 +52,15 @@ void Map::saveMarkers() const void Map::loadMarkers() { const auto filePath = path() + ".json"; - Files::File::checkAsync(filePath).then([this, filePath](Files::FileCheckResult &&result) { - if (!result.success() || !result.exists()) return; + Files::File::checkAsync(filePath).then([this, filePath](const Files::FileCheckResult &checkResult) { + if (!checkResult.success() || !checkResult.exists()) return; - Files::File::getDataAsync(filePath).then([this](Files::FileDataResult &&result) { - if (!result.success()) return; + Files::File::getDataAsync(filePath).then([this](const Files::FileDataResult &dataResult) { + if (!dataResult.success()) return; - auto markers = QJsonDocument::fromJson(result.data()).object()["markers"_L1].toArray(); + auto markers = QJsonDocument::fromJson(dataResult.data()).object()["markers"_L1].toArray(); - for (const auto &marker : markers) + foreach (const auto &marker, markers) { addMarker(new MapMarker(marker.toObject(), this)); } @@ -107,7 +107,7 @@ void MapCategory::loadMaps() const auto path = FileUtils::fileInDir(name(), SettingsManager::getPath(u"maps"_s)); - Files::File::listAsync(path, true, false).then([this, path](Files::FileListResult &&result) { + Files::File::listAsync(path, true, false).then([this, path](const Files::FileListResult &result) { if (!result.success()) return; foreach (const auto &file, result.files()) @@ -135,7 +135,7 @@ void MapListModel::insert(QObject *item) endInsertRows(); } -void MapListModel::remove(QObject *item) +void MapListModel::remove(const QObject *item) { for (int i = 0; i < m_items.size(); ++i) { @@ -165,12 +165,12 @@ void MapListModel::clear() } } -void MapListModel::setElements(QList elements) +void MapListModel::setElements(const QList &elements) { clear(); - for (int i = elements.size() - 1; i > -1; i--) + for (auto i = elements.size() - 1; i > -1; i--) { - insert(elements[i]); + insert(elements.at(i)); } } diff --git a/src/tools/maps/map.h b/src/tools/maps/map.h index 7a667542..a4fbf15a 100644 --- a/src/tools/maps/map.h +++ b/src/tools/maps/map.h @@ -69,7 +69,7 @@ class MapListModel : public QAbstractListModel return static_cast(m_items.size()); } - void setElements(QList elements); + void setElements(const QList &elements); void clear(); [[nodiscard]] auto elements() const -> QList @@ -83,7 +83,7 @@ class MapListModel : public QAbstractListModel public slots: void insert(QObject *item); - void remove(QObject *item); + void remove(const QObject *item); protected: [[nodiscard]] auto roleNames() const -> QHash override; diff --git a/src/tools/notes/markdownhighlighter.cpp b/src/tools/notes/markdownhighlighter.cpp index 1f2f1f47..2c39089c 100644 --- a/src/tools/notes/markdownhighlighter.cpp +++ b/src/tools/notes/markdownhighlighter.cpp @@ -62,9 +62,9 @@ void MarkdownHighlighter::applyRegexMatch(const QString &text, const QString &re { auto match = iterator.next(); - for (auto i = match.capturedStart(group); i < match.capturedEnd(group); i++) + for (auto i = static_cast(match.capturedStart(group)); i < match.capturedEnd(group); i++) { - setFormat(static_cast(i), 1, combineFormats(format(i), charFormat)); + setFormat(i, 1, combineFormats(format(i), charFormat)); } } } diff --git a/src/tools/notes/notessaveload.cpp b/src/tools/notes/notessaveload.cpp index 42edd120..465eea9c 100644 --- a/src/tools/notes/notessaveload.cpp +++ b/src/tools/notes/notessaveload.cpp @@ -57,7 +57,7 @@ void NotesSaveLoad::loadPages() qCDebug(gmNotesSaveLoad()) << "Loading pages in" << directory; - Files::File::listAsync(directory, true, false).then([this, chapter](Files::FileListResult &&result) { + Files::File::listAsync(directory, true, false).then([this, chapter](const Files::FileListResult &result) { if (!chapter) return; buildPages(result.files(), *chapter); }); diff --git a/src/tools/notes/notestool.cpp b/src/tools/notes/notestool.cpp index 1acc2e75..903f387e 100644 --- a/src/tools/notes/notestool.cpp +++ b/src/tools/notes/notestool.cpp @@ -99,7 +99,7 @@ void NotesTool::encrypt() /** * Export the current page as PDF document */ -void NotesTool::exportPdf() +void NotesTool::exportPdf() const { if (editMode() || !m_currentPage || !m_qmlTextDoc) return; diff --git a/src/tools/notes/notestool.h b/src/tools/notes/notestool.h index d3e36591..1b55c420 100644 --- a/src/tools/notes/notestool.h +++ b/src/tools/notes/notestool.h @@ -47,7 +47,7 @@ class NotesTool : public AbstractTool public slots: void loadData() override; void encrypt(); - void exportPdf(); + void exportPdf() const; void linkClicked(const QString &link) const; void createBook(const QString &name); diff --git a/src/tools/shop/itemeditor.cpp b/src/tools/shop/itemeditor.cpp index 83f3d8ee..0f71c17d 100644 --- a/src/tools/shop/itemeditor.cpp +++ b/src/tools/shop/itemeditor.cpp @@ -71,7 +71,7 @@ auto ItemEditor::addItem(const QString &name, const QString &price, const QStrin } else { - m_itemModel.insert(insert, item); + m_itemModel.insert(static_cast(insert), item); } madeChanges(); diff --git a/src/tools/shop/shoptool.cpp b/src/tools/shop/shoptool.cpp index 25ada61d..ad6b992b 100644 --- a/src/tools/shop/shoptool.cpp +++ b/src/tools/shop/shoptool.cpp @@ -28,7 +28,7 @@ void ShopTool::onCurrentProjectChanged(const ShopProject *project) m_projectConnection = connect(project, &ShopProject::currentShopChanged, this, &ShopTool::onCurrentShopChanged); if (!project->currentCategory()) return; - auto *shop = project->currentCategory()->currentShop(); + const auto *shop = project->currentCategory()->currentShop(); onCurrentShopChanged(shop); }