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

@@ -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<QString, QString> MappingTable;
-
     QString mapKey(const QString &key)
     {
-        static const MappingTable keyMapping = {
-
+        static const QHash<QString, QString> 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();
 }