Avoid base class call antipattern in classes derived from ContentLoader

apps/openmw/mwworld/contentloader.hpp

@@ -2,33 +2,20 @@
 #define CONTENTLOADER_HPP
 
 #include <boost/filesystem/path.hpp>
-#include <MyGUI_TextIterator.h>
 
-#include <components/debug/debuglog.hpp>
+namespace Loading
-#include "components/loadinglistener/loadinglistener.hpp"
+{
+    class Listener;
+}
 
 namespace MWWorld
 {
 
 struct ContentLoader
 {
-    ContentLoader(Loading::Listener& listener)
+    virtual ~ContentLoader() = default;
-      : mListener(listener)
-    {
-    }
 
-    virtual ~ContentLoader()
+    virtual void load(const boost::filesystem::path& filepath, int& index, Loading::Listener* listener) = 0;
-    {
-    }
-
-    virtual void load(const boost::filesystem::path& filepath, int& index)
-    {
-        Log(Debug::Info) << "Loading content file " << filepath.string();
-        mListener.setLabel(MyGUI::TextIterator::toTagsString(filepath.string()));
-    }
-
-    protected:
-        Loading::Listener& mListener;
 };
 
 } /* namespace MWWorld */

apps/openmw/mwworld/esmloader.cpp

@@ -27,25 +27,22 @@ namespace MWWorld
 {
 
 EsmLoader::EsmLoader(MWWorld::ESMStore& store, std::vector<ESM::ESMReader>& readers,
-  ToUTF8::Utf8Encoder* encoder, Loading::Listener& listener)
+    ToUTF8::Utf8Encoder* encoder)
-  : ContentLoader(listener)
+    : mEsm(readers)
-  , mEsm(readers)
+    , mStore(store)
-  , mStore(store)
+    , mEncoder(encoder)
-  , mEncoder(encoder)
 {
 }
 
-void EsmLoader::load(const boost::filesystem::path& filepath, int& index)
+void EsmLoader::load(const boost::filesystem::path& filepath, int& index, Loading::Listener* listener)
 {
-  ContentLoader::load(filepath.filename(), index);
+    ESM::ESMReader lEsm;
-
+    lEsm.setEncoder(mEncoder);
-  ESM::ESMReader lEsm;
+    lEsm.setIndex(index);
-  lEsm.setEncoder(mEncoder);
+    lEsm.open(filepath.string());
-  lEsm.setIndex(index);
+    lEsm.resolveParentFileIndices(mEsm);
-  lEsm.open(filepath.string());
+    mEsm[index] = lEsm;
-  lEsm.resolveParentFileIndices(mEsm);
+    mStore.load(mEsm[index], listener);
-  mEsm[index] = lEsm;
-  mStore.load(mEsm[index], &mListener);
 }
 
     void convertMagicEffects(ESM::CreatureStats& creatureStats, ESM::InventoryState& inventory, ESM::NpcStats* npcStats)

apps/openmw/mwworld/esmloader.hpp

@@ -26,14 +26,14 @@ class ESMStore;
 struct EsmLoader : public ContentLoader
 {
     EsmLoader(MWWorld::ESMStore& store, std::vector<ESM::ESMReader>& readers,
-      ToUTF8::Utf8Encoder* encoder, Loading::Listener& listener);
+        ToUTF8::Utf8Encoder* encoder);
 
-    void load(const boost::filesystem::path& filepath, int& index) override;
+    void load(const boost::filesystem::path& filepath, int& index, Loading::Listener* listener) override;
 
     private:
-      std::vector<ESM::ESMReader>& mEsm;
+        std::vector<ESM::ESMReader>& mEsm;
-      MWWorld::ESMStore& mStore;
+        MWWorld::ESMStore& mStore;
-      ToUTF8::Utf8Encoder* mEncoder;
+        ToUTF8::Utf8Encoder* mEncoder;
 };
 
 void convertMagicEffects(ESM::CreatureStats& creatureStats, ESM::InventoryState& inventory, ESM::NpcStats* npcStats = nullptr);

apps/openmw/mwworld/worldimp.cpp

@@ -7,6 +7,8 @@
 #include <BulletCollision/CollisionDispatch/btCollisionWorld.h>
 #include <BulletCollision/CollisionShapes/btCompoundShape.h>
 
+#include <MyGUI_TextIterator.h>
+
 #include <components/debug/debuglog.hpp>
 
 #include <components/esm/esmreader.hpp>
@@ -30,6 +32,8 @@
 #include <components/detournavigator/navigator.hpp>
 #include <components/detournavigator/settings.hpp>
 
+#include <components/loadinglistener/loadinglistener.hpp>
+
 #include "../mwbase/environment.hpp"
 #include "../mwbase/soundmanager.hpp"
 #include "../mwbase/mechanicsmanager.hpp"
@@ -79,43 +83,40 @@ namespace MWWorld
 {
     struct GameContentLoader : public ContentLoader
     {
-        GameContentLoader(Loading::Listener& listener)
+        void addLoader(std::string&& extension, ContentLoader& loader)
-          : ContentLoader(listener)
         {
+            mLoaders.emplace(std::move(extension), &loader);
         }
 
-        bool addLoader(const std::string& extension, ContentLoader* loader)
+        void load(const boost::filesystem::path& filepath, int& index, Loading::Listener* listener) override
         {
-            return mLoaders.insert(std::make_pair(extension, loader)).second;
+            const auto it = mLoaders.find(Misc::StringUtils::lowerCase(filepath.extension().string()));
-        }
-
-        void load(const boost::filesystem::path& filepath, int& index) override
-        {
-            LoadersContainer::iterator it(mLoaders.find(Misc::StringUtils::lowerCase(filepath.extension().string())));
             if (it != mLoaders.end())
             {
-                it->second->load(filepath, index);
+                const std::string filename = filepath.filename().string();
+                Log(Debug::Info) << "Loading content file " << filename;
+                if (listener != nullptr)
+                    listener->setLabel(MyGUI::TextIterator::toTagsString(filename));
+                it->second->load(filepath, index, listener);
             }
             else
             {
-              std::string msg("Cannot load file: ");
+                std::string msg("Cannot load file: ");
-              msg += filepath.string();
+                msg += filepath.string();
-              throw std::runtime_error(msg.c_str());
+                throw std::runtime_error(msg.c_str());
             }
         }
 
         private:
-          typedef std::map<std::string, ContentLoader*> LoadersContainer;
+            std::map<std::string, ContentLoader*> mLoaders;
-          LoadersContainer mLoaders;
     };
 
     struct OMWScriptsLoader : public ContentLoader
     {
         ESMStore& mStore;
-        OMWScriptsLoader(Loading::Listener& listener, ESMStore& store) : ContentLoader(listener), mStore(store) {}
+        OMWScriptsLoader(ESMStore& store) : mStore(store) {}
-        void load(const boost::filesystem::path& filepath, int& index) override
+        void load(const boost::filesystem::path& filepath, int& /*index*/, Loading::Listener* /*listener*/) override
         {
-            ContentLoader::load(filepath.filename(), index);
             mStore.addOMWScripts(filepath.string());
         }
     };
@@ -2936,18 +2937,18 @@ namespace MWWorld
 
     void World::loadContentFiles(const Files::Collections& fileCollections, const std::vector<std::string>& content, ESMStore& store, std::vector<ESM::ESMReader>& readers, ToUTF8::Utf8Encoder* encoder, Loading::Listener* listener)
     {
-        GameContentLoader gameContentLoader(*listener);
+        GameContentLoader gameContentLoader;
-        EsmLoader esmLoader(store, readers, encoder, *listener);
+        EsmLoader esmLoader(store, readers, encoder);
         validateMasterFiles(readers);
 
-        gameContentLoader.addLoader(".esm", &esmLoader);
+        gameContentLoader.addLoader(".esm", esmLoader);
-        gameContentLoader.addLoader(".esp", &esmLoader);
+        gameContentLoader.addLoader(".esp", esmLoader);
-        gameContentLoader.addLoader(".omwgame", &esmLoader);
+        gameContentLoader.addLoader(".omwgame", esmLoader);
-        gameContentLoader.addLoader(".omwaddon", &esmLoader);
+        gameContentLoader.addLoader(".omwaddon", esmLoader);
-        gameContentLoader.addLoader(".project", &esmLoader);
+        gameContentLoader.addLoader(".project", esmLoader);
 
-        OMWScriptsLoader omwScriptsLoader(*listener, store);
+        OMWScriptsLoader omwScriptsLoader(store);
-        gameContentLoader.addLoader(".omwscripts", &omwScriptsLoader);
+        gameContentLoader.addLoader(".omwscripts", omwScriptsLoader);
 
         int idx = 0;
         for (const std::string &file : content)
@@ -2956,7 +2957,7 @@ namespace MWWorld
             const Files::MultiDirCollection& col = fileCollections.getCollection(filename.extension().string());
             if (col.doesExist(file))
             {
-                gameContentLoader.load(col.getPath(file), idx);
+                gameContentLoader.load(col.getPath(file), idx, listener);
             }
             else
             {