Merge pull request #8558 from luis-pereira/containter-anti-pattern

Don't create temporary containers just to iterate over them

src/base/CMakeLists.txt

@@ -55,6 +55,7 @@ utils/net.h
 utils/random.h
 utils/string.h
 utils/version.h
+algorithm.h
 asyncfilestorage.h
 exceptions.h
 filesystemwatcher.h

src/base/algorithm.h

@@ -0,0 +1,41 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2018  Vladimir Golovnev <glassez@yandex.ru>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#pragma once
+
+namespace Dict
+{
+    // To be used with QMap, QHash and it's variants
+    template <typename Dictionary, typename BinaryPredicate>
+    void removeIf(Dictionary &&dict, BinaryPredicate p)
+    {
+        auto it = dict.begin();
+        while (it != dict.end())
+            it = (p(it.key(), it.value()) ? dict.erase(it) : it + 1);
+    }
+}

src/base/base.pri

@@ -1,4 +1,5 @@
 HEADERS += \
+    $$PWD/algorithm.h \
     $$PWD/asyncfilestorage.h \
     $$PWD/bittorrent/addtorrentparams.h  \
     $$PWD/bittorrent/cachestatus.h \

src/base/bittorrent/session.cpp

@@ -71,6 +71,7 @@
 #include <libtorrent/session_status.hpp>
 #include <libtorrent/torrent_info.hpp>
 
+#include "base/algorithm.h"
 #include "base/logger.h"
 #include "base/net/downloadhandler.h"
 #include "base/net/downloadmanager.h"
@@ -126,16 +127,16 @@ namespace
     QStringMap map_cast(const QVariantMap &map)
     {
         QStringMap result;
-        foreach (const QString &key, map.keys())
-            result[key] = map.value(key).toString();
+        for (auto i = map.cbegin(); i != map.cend(); ++i)
+            result[i.key()] = i.value().toString();
         return result;
     }
 
     QVariantMap map_cast(const QStringMap &map)
     {
         QVariantMap result;
-        foreach (const QString &key, map.keys())
-            result[key] = map.value(key);
+        for (auto i = map.cbegin(); i != map.cend(); ++i)
+            result[i.key()] = i.value();
         return result;
     }
 
@@ -197,7 +198,8 @@ namespace
     {
         QStringMap expanded = categories;
 
-        foreach (const QString &category, categories.keys()) {
+        for (auto i = categories.cbegin(); i != categories.cend(); ++i) {
+            const QString &category = i.key();
             foreach (const QString &subcat, Session::expandCategory(category)) {
                 if (!expanded.contains(subcat))
                     expanded[subcat] = "";
@@ -783,14 +785,16 @@ bool Session::removeCategory(const QString &name)
     bool result = false;
     if (isSubcategoriesEnabled()) {
         // remove subcategories
-        QString test = name + "/";
-        foreach (const QString &category, m_categories.keys()) {
+        const QString test = name + "/";
+        Dict::removeIf(m_categories, [this, &test, &result](const QString &category, const QString &)
+        {
             if (category.startsWith(test)) {
-                m_categories.remove(category);
                 result = true;
                 emit categoryRemoved(category);
+                return true;
             }
-        }
+            return false;
+        });
     }
 
     result = (m_categories.remove(name) > 0) || result;
@@ -2001,8 +2005,8 @@ void Session::decreaseTorrentsPriority(const QStringList &hashes)
         torrentQueue.pop();
     }
 
-    foreach (const InfoHash &hash, m_loadedMetadata.keys())
-        torrentQueuePositionBottom(m_nativeSession->find_torrent(hash));
+    for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i)
+        torrentQueuePositionBottom(m_nativeSession->find_torrent(i.key()));
 }
 
 void Session::topTorrentsPriority(const QStringList &hashes)
@@ -2046,8 +2050,8 @@ void Session::bottomTorrentsPriority(const QStringList &hashes)
         torrentQueue.pop();
     }
 
-    foreach (const InfoHash &hash, m_loadedMetadata.keys())
-        torrentQueuePositionBottom(m_nativeSession->find_torrent(hash));
+    for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i)
+        torrentQueuePositionBottom(m_nativeSession->find_torrent(i.key()));
 }
 
 QHash<InfoHash, TorrentHandle *> Session::torrents() const

src/base/filesystemwatcher.cpp

@@ -12,6 +12,7 @@
 #endif
 #endif
 
+#include "algorithm.h"
 #include "base/bittorrent/magneturi.h"
 #include "base/bittorrent/torrentinfo.h"
 #include "base/preferences.h"
@@ -145,22 +146,24 @@ void FileSystemWatcher::processPartialTorrents()
     QStringList noLongerPartial;
 
     // Check which torrents are still partial
-    foreach (const QString &torrentPath, m_partialTorrents.keys()) {
-        if (!QFile::exists(torrentPath)) {
-            m_partialTorrents.remove(torrentPath);
-        }
-        else if (BitTorrent::TorrentInfo::loadFromFile(torrentPath).isValid()) {
+    Dict::removeIf(m_partialTorrents, [&noLongerPartial](const QString &torrentPath, int &value)
+    {
+        if (!QFile::exists(torrentPath))
+            return true;
+
+        if (BitTorrent::TorrentInfo::loadFromFile(torrentPath).isValid()) {
             noLongerPartial << torrentPath;
-            m_partialTorrents.remove(torrentPath);
+            return true;
         }
-        else if (m_partialTorrents[torrentPath] >= MAX_PARTIAL_RETRIES) {
-            m_partialTorrents.remove(torrentPath);
+
+        if (value >= MAX_PARTIAL_RETRIES) {
             QFile::rename(torrentPath, torrentPath + ".invalid");
+            return true;
         }
-        else {
-            ++m_partialTorrents[torrentPath];
-        }
-    }
+
+        ++value;
+        return false;
+    });
 
     // Stop the partial timer if necessary
     if (m_partialTorrents.empty()) {

src/base/net/portforwarder.cpp

@@ -121,8 +121,10 @@ void PortForwarder::start()
     settingsPack.set_bool(libt::settings_pack::enable_natpmp, true);
     m_provider->apply_settings(settingsPack);
 #endif
-    foreach (quint16 port, m_mappedPorts.keys())
-        m_mappedPorts[port] = m_provider->add_port_mapping(libt::session::tcp, port, port);
+    for (auto i = m_mappedPorts.begin(); i != m_mappedPorts.end(); ++i) {
+        // quint16 port = i.key();
+        i.value() = m_provider->add_port_mapping(libt::session::tcp, i.key(), i.key());
+    }
     m_active = true;
     Logger::instance()->addMessage(tr("UPnP / NAT-PMP support [ON]"), Log::INFO);
 }

src/base/search/searchpluginmanager.cpp

@@ -100,7 +100,7 @@ QStringList SearchPluginManager::allPlugins() const
 QStringList SearchPluginManager::enabledPlugins() const
 {
     QStringList plugins;
-    foreach (const PluginInfo *plugin, m_plugins.values()) {
+    for (const PluginInfo *plugin : qAsConst(m_plugins)) {
         if (plugin->enabled)
             plugins << plugin->name;
     }
@@ -111,7 +111,7 @@ QStringList SearchPluginManager::enabledPlugins() const
 QStringList SearchPluginManager::supportedCategories() const
 {
     QStringList result;
-    foreach (const PluginInfo *plugin, m_plugins.values()) {
+    for (const PluginInfo *plugin : qAsConst(m_plugins)) {
         if (plugin->enabled) {
             foreach (QString cat, plugin->supportedCategories) {
                 if (!result.contains(cat))