From 0735d0fc2dfac29949a6141f75eb011edcbe2e30 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Mon, 14 Dec 2015 17:55:24 -0800 Subject: [PATCH] More cleanup of FactoryManager --- include/openspace/util/factorymanager.h | 46 +++++++++++++++++++---- include/openspace/util/factorymanager.inl | 14 +++---- modules/base/basemodule.cpp | 4 +- modules/newhorizons/newhorizonsmodule.cpp | 4 +- modules/newhorizons/util/decoder.cpp | 1 + src/engine/openspaceengine.cpp | 8 +++- src/scene/ephemeris.cpp | 1 + src/util/factorymanager.cpp | 19 +++++----- 8 files changed, 65 insertions(+), 32 deletions(-) diff --git a/include/openspace/util/factorymanager.h b/include/openspace/util/factorymanager.h index ce74515a86..55f6f67ce0 100644 --- a/include/openspace/util/factorymanager.h +++ b/include/openspace/util/factorymanager.h @@ -25,8 +25,11 @@ #ifndef __FACTORYMANAGER_H__ #define __FACTORYMANAGER_H__ +#include #include +#include + namespace openspace { /** @@ -36,6 +39,23 @@ namespace openspace { */ class FactoryManager { public: + /// This exception is thrown if the ghoul::TemplateFactory could not be found in the + /// #factory method + struct FactoryNotFoundError : public ghoul::RuntimeError { + /** + * Constructor for FactoryNotFoundError, the \p type is a human-readable (-ish) + * type descriptor for the type T for the TemplateFactory that could + * not be found. + * \param type The type T for the TemplateFactory + * that could not be found + * \pre \p type must not be empty + */ + explicit FactoryNotFoundError(std::string type); + + /// The type describing the ghoul::TemplateFactory that could not be found + std::string type; + }; + /** * Static initializer that initializes the static member. This needs to be done before * the FactoryManager can be used. @@ -63,20 +83,30 @@ public: */ static FactoryManager& ref(); - void addFactory(ghoul::TemplateFactoryBase* factory); + /** + * Adds the passed \p factory to the FactoryManager. Factories may only be added once. + * \param factory The ghoul::TemplateFactory to add to this FactoryManager + * \pre \p factory must not be nullptr + */ + void addFactory(std::unique_ptr factory); + /** + * This method provides access to all registered ghoul::TemplateFactory%s through + * their type. The method will always return a proper ghoul::TemplateFactory or throw + * an error if the appropriate ghoul::TemplateFactory was not registered. + * \tparam T The type that the requested ghoul::TemplateFactory should create + * \return The ghoul::TemplateFactory that will create the pass type T + * \throw FactoryNotFoundError If the requested ghoul::TemplateFactory could not be + * found + */ template ghoul::TemplateFactory* factory() const; private: - FactoryManager() = default; - ~FactoryManager(); + /// Singleton member for the Factory Manager + static FactoryManager* _manager; - FactoryManager(const FactoryManager& c) = delete; - - static FactoryManager* _manager; ///< Singleton member - - std::vector _factories; + std::vector> _factories; }; } // namespace openspace diff --git a/include/openspace/util/factorymanager.inl b/include/openspace/util/factorymanager.inl index f46eb55df7..7f9989f619 100644 --- a/include/openspace/util/factorymanager.inl +++ b/include/openspace/util/factorymanager.inl @@ -22,20 +22,16 @@ * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * ****************************************************************************************/ -#include - namespace openspace { template -ghoul::TemplateFactory* FactoryManager::factory() const -{ - for (ghoul::TemplateFactoryBase* factory : _factories) { +ghoul::TemplateFactory* FactoryManager::factory() const { + for (auto& factory : _factories) { if (factory->baseClassType() == typeid(T)) - return dynamic_cast*>(factory); + return dynamic_cast*>(factory.get()); } - LERRORC("FactoryManager", "Could not find factory for type '" << typeid(T).name() - << "'"); - return nullptr; + + throw FactoryNotFoundError(typeid(T).name()); } } // namespace openspace diff --git a/modules/base/basemodule.cpp b/modules/base/basemodule.cpp index 4d3939941d..17ab552544 100644 --- a/modules/base/basemodule.cpp +++ b/modules/base/basemodule.cpp @@ -56,8 +56,8 @@ BaseModule::BaseModule() {} void BaseModule::internalInitialize() { - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); + FactoryManager::ref().addFactory(std::make_unique>()); + FactoryManager::ref().addFactory(std::make_unique>()); auto fRenderable = FactoryManager::ref().factory(); ghoul_assert(fRenderable, "Renderable factory was not created"); diff --git a/modules/newhorizons/newhorizonsmodule.cpp b/modules/newhorizons/newhorizonsmodule.cpp index 2a3e46a03e..5b49283792 100644 --- a/modules/newhorizons/newhorizonsmodule.cpp +++ b/modules/newhorizons/newhorizonsmodule.cpp @@ -53,8 +53,8 @@ NewHorizonsModule::NewHorizonsModule() void NewHorizonsModule::internalInitialize() { ImageSequencer2::initialize(); - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); + FactoryManager::ref().addFactory(std::make_unique>()); + FactoryManager::ref().addFactory(std::make_unique>()); auto fRenderable = FactoryManager::ref().factory(); ghoul_assert(fRenderable, "No renderable factory existed"); diff --git a/modules/newhorizons/util/decoder.cpp b/modules/newhorizons/util/decoder.cpp index 2c68b90886..275a822f8b 100644 --- a/modules/newhorizons/util/decoder.cpp +++ b/modules/newhorizons/util/decoder.cpp @@ -24,6 +24,7 @@ #include #include +#include namespace { const std::string _loggerCat = "Decoder"; diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index a02cc07a64..34d29b9069 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -118,8 +118,12 @@ OpenSpaceEngine::OpenSpaceEngine(std::string programName, _interactionHandler->setPropertyOwner(_globalPropertyNamespace.get()); _globalPropertyNamespace->addPropertySubOwner(_interactionHandler.get()); FactoryManager::initialize(); - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); - FactoryManager::ref().addFactory(new ghoul::TemplateFactory); + FactoryManager::ref().addFactory( + std::make_unique>() + ); + FactoryManager::ref().addFactory( + std::make_unique>() + ); SpiceManager::initialize(); Time::initialize(); ghoul::systemcapabilities::SystemCapabilities::initialize(); diff --git a/src/scene/ephemeris.cpp b/src/scene/ephemeris.cpp index 2c2a47c3d9..62f8753316 100644 --- a/src/scene/ephemeris.cpp +++ b/src/scene/ephemeris.cpp @@ -24,6 +24,7 @@ #include #include +#include namespace { const std::string _loggerCat = "Ephemeris"; diff --git a/src/util/factorymanager.cpp b/src/util/factorymanager.cpp index e35db1fdc3..bd5ea85722 100644 --- a/src/util/factorymanager.cpp +++ b/src/util/factorymanager.cpp @@ -29,6 +29,13 @@ namespace openspace { FactoryManager* FactoryManager::_manager = nullptr; + +FactoryManager::FactoryNotFoundError::FactoryNotFoundError(std::string t) + : ghoul::RuntimeError("Could not find TemplateFactory for type '" + t + "'") + , type(std::move(t)) +{ + ghoul_assert(!type.empty(), "Type must not be empty"); +} void FactoryManager::initialize() { ghoul_assert(!_manager, "Factory Manager must not have been initialized"); @@ -50,15 +57,9 @@ FactoryManager& FactoryManager::ref() { return *_manager; } -FactoryManager::~FactoryManager() { - for (ghoul::TemplateFactoryBase* factory : _factories) - delete factory; - _factories.clear(); -} - -void FactoryManager::addFactory(ghoul::TemplateFactoryBase* factory) { - _factories.push_back(factory); - +void FactoryManager::addFactory(std::unique_ptr factory) { + ghoul_assert(factory, "Factory must not be nullptr"); + _factories.push_back(std::move(factory)); } } // namespace openspace