From 708b5a69046a04359bc790f2f04204fa376fec18 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 23 Apr 2018 01:42:25 +0800 Subject: [PATCH] Refactor SettingsStorage class Make use of (i.e. returning) QFile::rename operation status Make log message more verbose Add const Remove empty lines Inline typedef --- src/base/settingsstorage.cpp | 58 ++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/base/settingsstorage.cpp b/src/base/settingsstorage.cpp index a4c6aa8fb..9fcd84dff 100644 --- a/src/base/settingsstorage.cpp +++ b/src/base/settingsstorage.cpp @@ -62,15 +62,12 @@ namespace QString deserialize(const QString &name, QVariantHash &data); QString serialize(const QString &name, const QVariantHash &data); - QString m_name; + const QString m_name; }; - typedef QHash MappingTable; - QString mapKey(const QString &key) { - static const MappingTable keyMapping = { - + static const QHash keyMapping = { {"BitTorrent/Session/MaxRatioAction", "Preferences/Bittorrent/MaxRatioAction"}, {"BitTorrent/Session/DefaultSavePath", "Preferences/Downloads/SavePath"}, {"BitTorrent/Session/TempPath", "Preferences/Downloads/TempPath"}, @@ -147,7 +144,6 @@ namespace {"AddNewTorrentDialog/TopLevel", "Preferences/Downloads/NewAdditionDialogFront"}, {"State/BannedIPs", "Preferences/IPFilter/BannedIPs"} - }; return keyMapping.value(key, key); @@ -211,7 +207,7 @@ QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultV void SettingsStorage::storeValue(const QString &key, const QVariant &value) { - QString realKey = mapKey(key); + const QString realKey = mapKey(key); QWriteLocker locker(&m_lock); if (m_data.value(realKey) != value) { m_dirty = true; @@ -222,7 +218,7 @@ void SettingsStorage::storeValue(const QString &key, const QVariant &value) void SettingsStorage::removeValue(const QString &key) { - QString realKey = mapKey(key); + const QString realKey = mapKey(key); QWriteLocker locker(&m_lock); if (m_data.contains(realKey)) { m_dirty = true; @@ -234,25 +230,24 @@ void SettingsStorage::removeValue(const QString &key) QVariantHash TransactionalSettings::read() { QVariantHash res; - bool writeBackNeeded = false; - QString newPath = deserialize(m_name + QLatin1String("_new"), res); + + const QString newPath = deserialize(m_name + QLatin1String("_new"), res); if (!newPath.isEmpty()) { // "_new" file is NOT empty // This means that the PC closed either due to power outage // or because the disk was full. In any case the settings weren't transferred // in their final position. So assume that qbittorrent_new.ini/qbittorrent_new.conf // contains the most recent settings. - Logger::instance()->addMessage(QObject::tr("Detected unclean program exit. Using fallback file to restore settings."), Log::WARNING); - writeBackNeeded = true; + Logger::instance()->addMessage(QObject::tr("Detected unclean program exit. Using fallback file to restore settings: %1") + .arg(Utils::Fs::toNativePath(newPath)) + , Log::WARNING); + + Utils::Fs::forceRemove(newPath); + write(res); } else { deserialize(m_name, res); } - Utils::Fs::forceRemove(newPath); - - if (writeBackNeeded) - write(res); - return res; } @@ -263,7 +258,7 @@ bool TransactionalSettings::write(const QVariantHash &data) // between deleting the file and recreating it. This is a safety measure. // Write everything to qBittorrent_new.ini/qBittorrent_new.conf and if it succeeds // replace qBittorrent.ini/qBittorrent.conf with it. - QString newPath = serialize(m_name + QLatin1String("_new"), data); + const QString newPath = serialize(m_name + QLatin1String("_new"), data); if (newPath.isEmpty()) { Utils::Fs::forceRemove(newPath); return false; @@ -272,10 +267,9 @@ bool TransactionalSettings::write(const QVariantHash &data) QString finalPath = newPath; int index = finalPath.lastIndexOf("_new", -1, Qt::CaseInsensitive); finalPath.remove(index, 4); - Utils::Fs::forceRemove(finalPath); - QFile::rename(newPath, finalPath); - return true; + Utils::Fs::forceRemove(finalPath); + return QFile::rename(newPath, finalPath); } QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data) @@ -301,13 +295,19 @@ QString TransactionalSettings::serialize(const QString &name, const QVariantHash settings->setValue(i.key(), i.value()); settings->sync(); // Important to get error status - QSettings::Status status = settings->status(); - if (status != QSettings::NoError) { - if (status == QSettings::AccessError) - Logger::instance()->addMessage(QObject::tr("An access error occurred while trying to write the configuration file."), Log::CRITICAL); - else - Logger::instance()->addMessage(QObject::tr("A format error occurred while trying to write the configuration file."), Log::CRITICAL); - return QString(); + + switch (settings->status()) { + case QSettings::NoError: + return settings->fileName(); + case QSettings::AccessError: + Logger::instance()->addMessage(QObject::tr("An access error occurred while trying to write the configuration file."), Log::CRITICAL); + break; + case QSettings::FormatError: + Logger::instance()->addMessage(QObject::tr("A format error occurred while trying to write the configuration file."), Log::CRITICAL); + break; + default: + Logger::instance()->addMessage(QObject::tr("An unknown error occurred while trying to write the configuration file."), Log::CRITICAL); + break; } - return settings->fileName(); + return QString(); }