Skip to content

Commit

Permalink
STYLE: Remove pointer indirection from lists of ObjectFactoryBasePrivate
Browse files Browse the repository at this point in the history 
Removed unnecessary dynamic memory allocation from internal factory lists.
  • Loading branch information
@N-Dekker @dzenanz
N-Dekker authored and dzenanz committed Sep 23, 2022
1 parent f02dafa commit 48d0cb0
Showing 1 changed file with 56 additions and 89 deletions.
145 changes: 56 additions & 89 deletions Modules/Core/Common/src/itkObjectFactoryBase.cxx
View file
Expand Up @@ -48,27 +48,20 @@ using FactoryListType = std::list<itk::ObjectFactoryBase *>;
// duplicating factories that have already been registered and only add
// factories that were not already in the list.
void
SynchronizeList(FactoryListType * output, FactoryListType * input, bool internal)
SynchronizeList(FactoryListType & output, FactoryListType & input, bool internal)
{
if (!input)
{
return;
}
for (auto & factory : *input)
for (auto & factory : input)
{
int pos = -1;
if (output)
int curr = 0;
for (auto & i : output)
{
int curr = 0;
for (auto & i : *output)
if (typeid(*i) == typeid(*factory))
{
if (typeid(*i) == typeid(*factory))
{
pos = curr;
break; // factory already in internal factories.
}
++curr;
pos = curr;
break; // factory already in internal factories.
}
++curr;
}
if (pos == -1)
{
Expand Down Expand Up @@ -107,23 +100,18 @@ class ObjectFactoryBasePrivate : public LightObject
~ObjectFactoryBasePrivate() override
{
itk::ObjectFactoryBase::UnRegisterAllFactories();
if (m_InternalFactories)
for (auto & m_InternalFactorie : m_InternalFactories)
{
for (auto & m_InternalFactorie : *m_InternalFactories)
{
m_InternalFactorie->UnRegister();
}
delete m_InternalFactories;
m_InternalFactories = nullptr;
m_InternalFactorie->UnRegister();
}
}

ObjectFactoryBasePrivate() = default;

std::list<itk::ObjectFactoryBase *> * m_RegisteredFactories{ nullptr };
std::list<itk::ObjectFactoryBase *> * m_InternalFactories{ nullptr };
bool m_Initialized{ false };
bool m_StrictVersionChecking{ false };
FactoryListType m_RegisteredFactories{};
FactoryListType m_InternalFactories{};
bool m_Initialized{ false };
bool m_StrictVersionChecking{ false };
};

ObjectFactoryBasePrivate *
Expand Down Expand Up @@ -202,7 +190,7 @@ ObjectFactoryBase::CreateInstance(const char * itkclassname)
{
ObjectFactoryBase::Initialize();

for (auto & registeredFactory : *m_PimplGlobals->m_RegisteredFactories)
for (auto & registeredFactory : m_PimplGlobals->m_RegisteredFactories)
{
LightObject::Pointer newobject = registeredFactory->CreateObject(itkclassname);
if (newobject)
Expand All @@ -220,7 +208,7 @@ ObjectFactoryBase::CreateAllInstance(const char * itkclassname)
ObjectFactoryBase::Initialize();

std::list<LightObject::Pointer> created;
for (auto & registeredFactory : *m_PimplGlobals->m_RegisteredFactories)
for (auto & registeredFactory : m_PimplGlobals->m_RegisteredFactories)
{
std::list<LightObject::Pointer> moreObjects = registeredFactory->CreateAllObject(itkclassname);
created.splice(created.end(), moreObjects);
Expand All @@ -235,19 +223,6 @@ void
ObjectFactoryBase::InitializeFactoryList()
{
itkInitGlobalsMacro(PimplGlobals);

/**
* Don't do anything if we are already initialized
*/
if (!m_PimplGlobals->m_RegisteredFactories)
{
m_PimplGlobals->m_RegisteredFactories = new FactoryListType;
}

if (!m_PimplGlobals->m_InternalFactories)
{
m_PimplGlobals->m_InternalFactories = new FactoryListType;
}
}

/**
Expand All @@ -258,7 +233,7 @@ ObjectFactoryBase::Initialize()
{
itkInitGlobalsMacro(PimplGlobals);

if (!m_PimplGlobals->m_Initialized || !m_PimplGlobals->m_RegisteredFactories)
if (!m_PimplGlobals->m_Initialized)
{
m_PimplGlobals->m_Initialized = true;
ObjectFactoryBase::InitializeFactoryList();
Expand All @@ -279,14 +254,14 @@ ObjectFactoryBase::RegisterInternal()
itkInitGlobalsMacro(PimplGlobals);

// Guarantee that no internal factories have already been registered.
itkAssertInDebugAndIgnoreInReleaseMacro(m_PimplGlobals->m_RegisteredFactories->empty());
m_PimplGlobals->m_RegisteredFactories->clear();
itkAssertInDebugAndIgnoreInReleaseMacro(m_PimplGlobals->m_RegisteredFactories.empty());
m_PimplGlobals->m_RegisteredFactories.clear();

// Register all factories registered by the
// "RegisterFactoryInternal" method
for (auto & internalFactory : *m_PimplGlobals->m_InternalFactories)
for (auto & internalFactory : m_PimplGlobals->m_InternalFactories)
{
m_PimplGlobals->m_RegisteredFactories->push_back(internalFactory);
m_PimplGlobals->m_RegisteredFactories.push_back(internalFactory);
}
}

Expand Down Expand Up @@ -542,12 +517,12 @@ ObjectFactoryBase::RegisterFactoryInternal(ObjectFactoryBase * factory)
// libraries to be loaded and this method is called during static
// initialization.
ObjectFactoryBase::InitializeFactoryList();
m_PimplGlobals->m_InternalFactories->push_back(factory);
m_PimplGlobals->m_InternalFactories.push_back(factory);
factory->Register();
// if the internal factories have already been register add this one too
if (m_PimplGlobals->m_Initialized)
{
m_PimplGlobals->m_RegisteredFactories->push_back(factory);
m_PimplGlobals->m_RegisteredFactories.push_back(factory);
}
}

Expand All @@ -567,7 +542,7 @@ ObjectFactoryBase::RegisterFactory(ObjectFactoryBase * factory, InsertionPositio
else
{
// Factories must only be loaded once
for (auto & registeredFactory : *m_PimplGlobals->m_RegisteredFactories)
for (auto & registeredFactory : m_PimplGlobals->m_RegisteredFactories)
{
if (registeredFactory->m_LibraryPath == factory->m_LibraryPath)
{
Expand Down Expand Up @@ -609,7 +584,7 @@ ObjectFactoryBase::RegisterFactory(ObjectFactoryBase * factory, InsertionPositio
itkGenericExceptionMacro(
<< "position argument must not be used with InsertionPositionEnum::INSERT_AT_BACK option");
}
m_PimplGlobals->m_RegisteredFactories->push_back(factory);
m_PimplGlobals->m_RegisteredFactories.push_back(factory);
break;
}
case InsertionPositionEnum::INSERT_AT_FRONT:
Expand All @@ -619,21 +594,21 @@ ObjectFactoryBase::RegisterFactory(ObjectFactoryBase * factory, InsertionPositio
itkGenericExceptionMacro(
<< "position argument must not be used with InsertionPositionEnum::INSERT_AT_FRONT option");
}
m_PimplGlobals->m_RegisteredFactories->push_front(factory);
m_PimplGlobals->m_RegisteredFactories.push_front(factory);
break;
}
case InsertionPositionEnum::INSERT_AT_POSITION:
{
const size_t numberOfFactories = m_PimplGlobals->m_RegisteredFactories->size();
const size_t numberOfFactories = m_PimplGlobals->m_RegisteredFactories.size();
if (position < numberOfFactories)
{
auto fitr = m_PimplGlobals->m_RegisteredFactories->begin();
auto fitr = m_PimplGlobals->m_RegisteredFactories.begin();
while (position--)
{
++fitr;
}

m_PimplGlobals->m_RegisteredFactories->insert(fitr, factory);
m_PimplGlobals->m_RegisteredFactories.insert(fitr, factory);
break;
}
else
Expand Down Expand Up @@ -681,8 +656,8 @@ ObjectFactoryBase::DeleteNonInternalFactory(ObjectFactoryBase * factory)
itkInitGlobalsMacro(PimplGlobals);

// if factory is not internal then delete
if (std::find(m_PimplGlobals->m_InternalFactories->begin(), m_PimplGlobals->m_InternalFactories->end(), factory) ==
m_PimplGlobals->m_InternalFactories->end())
if (std::find(m_PimplGlobals->m_InternalFactories.begin(), m_PimplGlobals->m_InternalFactories.end(), factory) ==
m_PimplGlobals->m_InternalFactories.end())
{
factory->UnRegister();
}
Expand All @@ -696,17 +671,13 @@ ObjectFactoryBase::UnRegisterFactory(ObjectFactoryBase * factory)
{
itkInitGlobalsMacro(PimplGlobals);

if (m_PimplGlobals->m_RegisteredFactories)
for (auto i = m_PimplGlobals->m_RegisteredFactories.begin(); i != m_PimplGlobals->m_RegisteredFactories.end(); ++i)
{
for (auto i = m_PimplGlobals->m_RegisteredFactories->begin(); i != m_PimplGlobals->m_RegisteredFactories->end();
++i)
if (factory == *i)
{
if (factory == *i)
{
DeleteNonInternalFactory(factory);
m_PimplGlobals->m_RegisteredFactories->remove(factory);
return;
}
DeleteNonInternalFactory(factory);
m_PimplGlobals->m_RegisteredFactories.remove(factory);
return;
}
}
}
Expand All @@ -719,34 +690,30 @@ ObjectFactoryBase::UnRegisterAllFactories()
{
itkInitGlobalsMacro(PimplGlobals);

if (m_PimplGlobals->m_RegisteredFactories)
// Collect up all the library handles so they can be closed
// AFTER the factory has been deleted.
std::list<void *> libs;
for (auto & registeredFactory : m_PimplGlobals->m_RegisteredFactories)
{
// Collect up all the library handles so they can be closed
// AFTER the factory has been deleted.
std::list<void *> libs;
for (auto & registeredFactory : *m_PimplGlobals->m_RegisteredFactories)
{
libs.push_back(static_cast<void *>(registeredFactory->m_LibraryHandle));
}
// Unregister each factory
for (auto & registeredFactory : *m_PimplGlobals->m_RegisteredFactories)
{
DeleteNonInternalFactory(registeredFactory);
}
libs.push_back(static_cast<void *>(registeredFactory->m_LibraryHandle));
}
// Unregister each factory
for (auto & registeredFactory : m_PimplGlobals->m_RegisteredFactories)
{
DeleteNonInternalFactory(registeredFactory);
}
#ifdef ITK_DYNAMIC_LOADING
// And delete the library handles all at once
for (auto & lib : libs)
// And delete the library handles all at once
for (auto & lib : libs)
{
if (lib)
{
if (lib)
{
DynamicLoader::CloseLibrary(static_cast<LibHandle>(lib));
}
DynamicLoader::CloseLibrary(static_cast<LibHandle>(lib));
}
#endif
delete m_PimplGlobals->m_RegisteredFactories;
m_PimplGlobals->m_RegisteredFactories = nullptr;
m_PimplGlobals->m_Initialized = false;
}
#endif
m_PimplGlobals->m_RegisteredFactories.clear();
m_PimplGlobals->m_Initialized = false;
}

/**
Expand Down Expand Up @@ -891,7 +858,7 @@ ObjectFactoryBase::GetRegisteredFactories()
// static SingletonIndex * singletonIndex = SingletonIndex::GetInstance();
// Unused(singletonIndex);
ObjectFactoryBase::Initialize();
return *m_PimplGlobals->m_RegisteredFactories;
return m_PimplGlobals->m_RegisteredFactories;
}

/**
Expand Down

0 comments on commit 48d0cb0

Please sign in to comment.