Fixed lots of minor code smells

PhilInTheGaps · Nov 6, 2023 · 0975e65 · 0975e65
1 parent 6161d63
commit 0975e65
Show file tree

Hide file tree

Showing 139 changed files with 622 additions and 681 deletions.
diff --git a/app/logger.cpp b/app/logger.cpp
@@ -9,7 +9,7 @@
 
 Q_LOGGING_CATEGORY(gmLogger, "gm.logger")
 
-static constexpr const char *RELATIVE_LOGFILE_PATH = ".gm-companion/log.txt";
+static constexpr auto RELATIVE_LOGFILE_PATH = ".gm-companion/log.txt";
 
 Logger::Logger()
 {
@@ -30,14 +30,6 @@ Logger::Logger()
     qInstallMessageHandler(messageHandler);
 }
 
-Logger::~Logger()
-{
-    if (m_logFile.isOpen())
-    {
-        m_logFile.close();
-    }
-}
-
 void Logger::messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg)
 {
     auto timestamp = QDateTime::currentDateTime();

diff --git a/app/logger.h b/app/logger.h
@@ -9,18 +9,16 @@ class Logger
 {
 public:
     Logger();
-    ~Logger();
-    Q_DISABLE_COPY_MOVE(Logger);
 
     static void messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg);
 
 private:
-    inline static QFile m_logFile;
+    QFile m_logFile;
     inline static QTextStream m_logStream;
     inline static QMutex m_logMutex;
 
     static void createLogFileDir(const QString &filePath);
-    static void clearOldLog();
+    void clearOldLog();
 
     static auto msgTypeToPrefix(QtMsgType type) -> QString;
 };
diff --git a/app/ui/FileDialog/filedialogbackend.cpp b/app/ui/FileDialog/filedialogbackend.cpp
@@ -105,7 +105,7 @@ void FileDialogBackend::createFolder(const QString &folderName)
 {
     const auto path = FileUtils::fileInDir(folderName, currentDir());
 
-    File::createDirAsync(path).then([this](FileResult &&result) {
+    File::createDirAsync(path).then([this](const FileResult &result) {
         if (!result.success())
         {
             qCWarning(gmFileDialog()) << result.errorMessage();
@@ -153,7 +153,7 @@ void FileDialogBackend::stopCurrentRequest()
     }
 }
 
-void FileDialogBackend::onFileListReceived(FileListResult &&result)
+void FileDialogBackend::onFileListReceived(const FileListResult &result)
 {
     auto countBefore = a_entries.count();
 

diff --git a/app/ui/FileDialog/filedialogbackend.h b/app/ui/FileDialog/filedialogbackend.h
@@ -55,7 +55,7 @@ public slots:
     void stopCurrentRequest();
     void clearForward();
 
-    void onFileListReceived(FileListResult &&result);
+    void onFileListReceived(const FileListResult &result);
 
 private slots:
     void onCurrentDirChanged(const QString &dir);

diff --git a/app/ui/tools/generators/NameGenerator.qml b/app/ui/tools/generators/NameGenerator.qml
@@ -68,8 +68,8 @@ Page {
 
                         buttonText: modelData.name
 
-                        anchors.left: parent.left
-                        anchors.right: parent.right
+                        anchors.left: parent ? parent.left : undefined
+                        anchors.right: parent ? parent.right : undefined
 
                         onClicked: {
                             if (NameGeneratorTool.loadGenerator(index)) {

diff --git a/src/addons/addonrepositorymanager.cpp b/src/addons/addonrepositorymanager.cpp
@@ -82,14 +82,15 @@ void AddonRepositoryManager::fetchAllRepositoryData()
 
     foreach (const auto *repo, repositories())
     {
-        auto future = fetchRepositoryDataAsync(repo->url()).then([this, repo](std::vector<AddonReleaseInfo> &&info) {
-            if (!info.empty())
-            {
-                qCDebug(gmAddonRepoManager()) << "Successfully read addon repository" << repo->url();
-            }
+        auto future =
+            fetchRepositoryDataAsync(repo->url()).then([this, repo](const std::vector<AddonReleaseInfo> &info) {
+                if (!info.empty())
+                {
+                    qCDebug(gmAddonRepoManager()) << "Successfully read addon repository" << repo->url();
+                }
 
-            m_releaseInfos.insert(m_releaseInfos.end(), info.begin(), info.end());
-        });
+                m_releaseInfos.insert(m_releaseInfos.end(), info.begin(), info.end());
+            });
 
         combinator << future;
     }
@@ -213,10 +214,10 @@ auto AddonRepositoryManager::parseRepositoryData(const QByteArray &data) -> std:
         auto release = getNewestCompatibleRelease(entry["releases"_L1].toArray());
         if (release.isEmpty()) continue;
 
-        result.push_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(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()));
     }
 
     return result;

diff --git a/src/common/models/customobjectlistmodel.cpp b/src/common/models/customobjectlistmodel.cpp
@@ -9,7 +9,7 @@ CustomObjectListModel::CustomObjectListModel(bool isOwning, QObject *parent)
 auto CustomObjectListModel::rowCount(const QModelIndex &parent) const -> int
 {
     Q_UNUSED(parent)
-    return m_objects.size();
+    return static_cast<int>(m_objects.size());
 }
 
 auto CustomObjectListModel::headerData(int section, Qt::Orientation orientation, int role) const -> QVariant
@@ -55,9 +55,9 @@ void CustomObjectListModel::replaceAll(const QList<QObject *> &objects)
 auto CustomObjectListModel::removeRows(int row, int count, const QModelIndex &parent) -> bool
 {
     const auto firstIndex = row;
-    auto lastIndex = row + count - 1;
 
-    if (Utils::isInBounds(m_objects, firstIndex) && Utils::isInBounds(m_objects, lastIndex))
+    if (auto lastIndex = row + count - 1;
+        Utils::isInBounds(m_objects, firstIndex) && Utils::isInBounds(m_objects, lastIndex))
     {
         beginRemoveRows(parent, firstIndex, lastIndex);
 
@@ -100,7 +100,7 @@ void CustomObjectListModel::append(QObject *object, const QModelIndex &parent)
 {
     takeOwnershipIfRequired(object);
 
-    const auto lastIndex = m_objects.size();
+    const auto lastIndex = static_cast<int>(m_objects.size());
     beginInsertRows(parent, lastIndex, lastIndex);
     m_objects.append(object);
     endInsertRows();

diff --git a/src/common/models/customobjectlistmodel.h b/src/common/models/customobjectlistmodel.h
@@ -16,7 +16,7 @@ class CustomObjectListModel : public QAbstractListModel
     explicit CustomObjectListModel(bool isOwning, QObject *parent = nullptr);
 
     [[nodiscard]] auto rowCount(const QModelIndex &parent = QModelIndex()) const -> int override;
-    [[nodiscard]] virtual auto data(const QModelIndex &index, int role) const -> QVariant override = 0;
+    [[nodiscard]] auto data(const QModelIndex &index, int role) const -> QVariant override = 0;
     [[nodiscard]] auto headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const
         -> QVariant override;
 

diff --git a/src/common/models/customsharedptrlistmodel.h b/src/common/models/customsharedptrlistmodel.h
@@ -22,7 +22,7 @@ template <class T> class CustomSharedPtrListModel : public QAbstractListModel
     [[nodiscard]] auto rowCount(const QModelIndex &parent = QModelIndex()) const -> int override
     {
         Q_UNUSED(parent)
-        return m_objects.size();
+        return static_cast<int>(m_objects.size());
     }
 
     [[nodiscard]] auto headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const
@@ -51,9 +51,9 @@ template <class T> class CustomSharedPtrListModel : public QAbstractListModel
     auto removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) -> bool override
     {
         const auto firstIndex = row;
-        auto lastIndex = row + count - 1;
 
-        if (Utils::isInBounds(m_objects, firstIndex) && Utils::isInBounds(m_objects, lastIndex))
+        if (auto lastIndex = row + count - 1;
+            Utils::isInBounds(m_objects, firstIndex) && Utils::isInBounds(m_objects, lastIndex))
         {
             beginRemoveRows(parent, firstIndex, lastIndex);
             m_objects.erase(m_objects.begin() + firstIndex, m_objects.begin() + firstIndex + count);

diff --git a/src/common/models/treeitem.cpp b/src/common/models/treeitem.cpp
@@ -6,7 +6,7 @@ using namespace Qt::Literals::StringLiterals;
 TreeItem::TreeItem(const QString &name, int depth, bool canToggle, QObject *parent)
     : BaseProjectItem(name, parent), a_canToggle(canToggle), a_depth(depth)
 {
-    if (auto *parentItem = qobject_cast<TreeItem *>(parent); parentItem)
+    if (const QPointer parentItem = qobject_cast<TreeItem *>(parent); parentItem)
     {
         connect(this, &TreeItem::childItemsChanged, parentItem, &TreeItem::childItemsChanged);
         connect(this, &TreeItem::destroyed, parentItem, &TreeItem::childItemsChanged);
@@ -132,7 +132,6 @@ void TreeItem::onChildIsCheckedChanged()
                 emit isCheckedChanged();
             }
             return;
-        case CheckedState::Checked:
         default:
             checkedChildren++;
             break;

diff --git a/src/common/settings/quicksettingsmanager.cpp b/src/common/settings/quicksettingsmanager.cpp
@@ -18,7 +18,7 @@ void QuickSettingsManager::setPath(const QString &path, const QString &value)
     SettingsManager::setPath(path, value);
 }
 
-auto QuickSettingsManager::getLanguageIndex() -> int
+auto QuickSettingsManager::getLanguageIndex() -> qsizetype
 {
     auto language = SettingsManager::getLanguageString();
     auto languages = getLanguageNames();

diff --git a/src/common/settings/quicksettingsmanager.h b/src/common/settings/quicksettingsmanager.h
@@ -75,7 +75,7 @@ class QuickSettingsManager : public QObject
     Q_INVOKABLE static QString getPath(const QString &path);
     Q_INVOKABLE static void setPath(const QString &path, const QString &value);
 
-    Q_INVOKABLE static int getLanguageIndex();
+    Q_INVOKABLE static qsizetype getLanguageIndex();
     Q_INVOKABLE static QStringList getLanguageNames();
     Q_INVOKABLE void setLanguage(const QString &language);
 

diff --git a/src/common/settings/settingsmanager.cpp b/src/common/settings/settingsmanager.cpp
@@ -119,12 +119,14 @@ void SettingsManager::setServerUrl(const QString &url, const QString &service)
 
 auto SettingsManager::getPassword(const QString &username, const QString &service) -> QString
 {
+    using namespace QKeychain;
+
     // job is deleted automatically when finished
-    auto *job = new QKeychain::ReadPasswordJob(u"gm-companion.%1"_s.arg(service));
+    auto *job = new ReadPasswordJob(u"gm-companion.%1"_s.arg(service));
     job->setKey(username);
 
     QEventLoop loop;
-    job->connect(job, &QKeychain::ReadPasswordJob::finished, &loop, &QEventLoop::quit);
+    connect(job, &ReadPasswordJob::finished, &loop, &QEventLoop::quit);
     job->start();
     loop.exec();
 
@@ -140,13 +142,15 @@ auto SettingsManager::getPassword(const QString &username, const QString &servic
 
 auto SettingsManager::setPassword(const QString &username, const QString &password, const QString &service) -> bool
 {
+    using namespace QKeychain;
+
     // job is deleted automatically when finished
-    auto *job = new QKeychain::WritePasswordJob(u"gm-companion.%1"_s.arg(service));
+    auto *job = new WritePasswordJob(u"gm-companion.%1"_s.arg(service));
     job->setKey(username);
     job->setTextData(password);
 
     QEventLoop loop;
-    job->connect(job, &QKeychain::WritePasswordJob::finished, &loop, &QEventLoop::quit);
+    connect(job, &WritePasswordJob::finished, &loop, &QEventLoop::quit);
     job->start();
     loop.exec();
 
@@ -173,11 +177,9 @@ auto SettingsManager::getDefaultPath(const QString &setting, const QString &grou
 /// Default value is PATHS_GROUP.
 auto SettingsManager::getActivePathGroup() -> QString
 {
-    auto cloudMode = instance()->get<QString>(u"cloudMode"_s, u"local"_s);
-
-    if (cloudMode == "GoogleDrive"_L1) return cloudMode;
-
-    if (cloudMode == "NextCloud"_L1) return cloudMode;
+    if (auto cloudMode = instance()->get<QString>(u"cloudMode"_s, u"local"_s);
+        cloudMode == "GoogleDrive"_L1 || cloudMode == "NextCloud"_L1)
+        return cloudMode;
 
     return PATHS_GROUP;
 }
@@ -203,9 +205,7 @@ auto SettingsManager::getIsAddonEnabled(const QString &addon) -> bool
 /// Updates the settings if something changed from a previous version
 void SettingsManager::updateSettings()
 {
-    const auto cloudMode = get(u"cloudMode"_s, u"0"_s);
-
-    if (cloudMode == "0"_L1)
+    if (const auto cloudMode = get(u"cloudMode"_s, u"0"_s); cloudMode == "0"_L1)
     {
         set(u"cloudMode"_s, u"local"_s);
     }

diff --git a/src/common/utils/fileutils.cpp b/src/common/utils/fileutils.cpp
@@ -94,10 +94,10 @@ auto FileUtils::incrementFileName(const QString &fileName) -> QString
 {
     if (fileName.isEmpty()) return u""_s;
 
-    auto nameAndSuffix = splitFileNameAndSuffix(fileName);
-    auto incrementedName = incrementName(nameAndSuffix.first);
+    auto [name, suffix] = splitFileNameAndSuffix(fileName);
+    auto incrementedName = incrementName(name);
 
-    return incrementedName + '.' + nameAndSuffix.second;
+    return incrementedName + '.' + suffix;
 }
 
 auto FileUtils::incrementName(const QString &name) -> QString

diff --git a/src/common/utils/stringutils.cpp b/src/common/utils/stringutils.cpp
@@ -40,12 +40,12 @@ auto StringUtils::rot13(const QString &input) -> QString
     {
         if (lowCaps.contains(character))
         {
-            const int index = lowCaps.indexOf(character);
+            const auto index = lowCaps.indexOf(character);
             encrypted.append(lowCaps.at(index + ROT13_PLACES));
         }
         else if (upperCaps.contains(character))
         {
-            const int index = upperCaps.indexOf(character);
+            const auto index = upperCaps.indexOf(character);
             encrypted.append(upperCaps.at(index + ROT13_PLACES));
         }
         else

diff --git a/src/common/utils/utils.h b/src/common/utils/utils.h
@@ -28,7 +28,7 @@ class Utils
 
     template <typename T> static auto isInBounds(const T &list, qsizetype index) -> bool
     {
-        return index > -1 && index < list.size();
+        return index > -1 && index < std::size(list);
     }
 
     template <typename T> static auto copyList(const QList<T *> &original) -> QList<T *>

diff --git a/src/filesystem/cache/filecache.cpp b/src/filesystem/cache/filecache.cpp
@@ -88,7 +88,7 @@ auto FileCache::copyEntry(const QString &path, const QString &copy) -> bool
     return false;
 }
 
-auto FileCache::checkEntry(const QString &path) -> bool
+auto FileCache::checkEntry(const QString &path) const -> bool
 {
     return m_entries.contains(path) && m_entries.value(path)->isFresh(m_expirationTimeMs);
 }
diff --git a/src/filesystem/cache/filecache.h b/src/filesystem/cache/filecache.h
@@ -16,7 +16,7 @@ class FileCache
     auto removeEntry(const QString &path) -> bool;
     auto moveEntry(const QString &oldPath, const QString &newPath) -> bool;
     auto copyEntry(const QString &path, const QString &copy) -> bool;
-    auto checkEntry(const QString &path) -> bool;
+    auto checkEntry(const QString &path) const -> bool;
 
     void printEntries() const;
 

diff --git a/src/filesystem/fileaccess.h b/src/filesystem/fileaccess.h
@@ -17,6 +17,7 @@ class FileAccess
 {
 public:
     FileAccess() = default;
+    virtual ~FileAccess() = default;
     Q_DISABLE_COPY_MOVE(FileAccess);
 
     virtual auto getDataAsync(const QString &path, bool allowCache) -> QFuture<FileDataResult> = 0;