Refactor abstract factories to centralize item registration

ctkAbsractLibraryFactory::registerLibrary(),
ctkAbsractLibraryFactory::registerQLibrary()and
ctkAbsractPluginFactory::registerLibrary() are now factorized into
ctkAbsractFileBasedFactory::registerFileItem()

There is nothing to prevent copy constructors nor no need to explicitly
require instanciation of factories -> Remove the explicitly defined default
constructors.

Libs/Core/Testing/Cpp/ctkAbstractLibraryFactoryTest1.cpp

@@ -47,11 +47,6 @@ public:
 //-----------------------------------------------------------------------------
 class ObjectFactoryItem : public ctkFactoryLibraryItem<Object>
 {
-public:
-  ObjectFactoryItem(const QString& path)
-    :ctkFactoryLibraryItem<Object>(path)
-  {
-  }
 protected:
   virtual Object* instanciator(){return new Object;}
 };
@@ -66,9 +61,9 @@ public:
   }
   
 protected:
-  virtual ctkFactoryLibraryItem<Object>* createFactoryLibraryItem(const QFileInfo& file)const
+  virtual ctkAbstractFactoryItem<Object>* createFactoryFileBasedItem()
   {
-    return new ObjectFactoryItem(file.filePath());
+    return new ObjectFactoryItem();
   }
 };
 

Libs/Core/Testing/Cpp/ctkAbstractPluginFactoryTest1.cpp

@@ -63,7 +63,7 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
   ctkAbstractPluginFactory< ctkDummyPlugin > pluginFactory;
   pluginFactory.setVerbose(true);
 
-  bool res = pluginFactory.registerLibrary("fail", QFileInfo("foo/bar.txt"));
+  bool res = pluginFactory.registerFileItem("fail", QFileInfo("foo/bar.txt"));
   if (res)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() registered bad file"
@@ -71,7 +71,7 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
     return EXIT_FAILURE;
     }
   
-  res = pluginFactory.registerLibrary("lib", file);
+  res = pluginFactory.registerFileItem("lib", file);
   if (!res || pluginFactory.keys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed"
@@ -79,7 +79,7 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
     return EXIT_FAILURE;
     }
   // register twice must return false
-  res = pluginFactory.registerLibrary("lib", file);
+  res = pluginFactory.registerFileItem("lib", file);
   if (res || pluginFactory.keys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed"
@@ -106,7 +106,7 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
   ctkAbstractPluginFactory< QPushButton > buttonPluginFactory;
   buttonPluginFactory.setVerbose(true);
   // it should register but fail while instanciating
-  res = buttonPluginFactory.registerLibrary("foo", file);
+  res = buttonPluginFactory.registerFileItem("foo", file);
   if (!res || buttonPluginFactory.keys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed" << std::endl;

Libs/Core/ctkAbstractFactory.h

@@ -42,7 +42,8 @@ template<typename BaseClassType>
 class ctkAbstractFactoryItem
 {
 public:
-  explicit ctkAbstractFactoryItem();
+  //explicit ctkAbstractFactoryItem();
+  ctkAbstractFactoryItem();
   
   virtual QString loadErrorString()const;
   virtual bool load() = 0;
@@ -78,7 +79,7 @@ public:
   typedef QHash<QString, QSharedPointer<ctkAbstractFactoryItem<BaseClassType> > > HashType;
 
   /// Constructor/Desctructor
-  explicit ctkAbstractFactory();
+  ctkAbstractFactory();
   virtual ~ctkAbstractFactory();
   virtual void printAdditionalInfo();
 
@@ -122,9 +123,10 @@ protected:
   typedef typename HashType::iterator       Iterator;
 
 private:
+  /*
   ctkAbstractFactory(const ctkAbstractFactory &); /// Not implemented
   void operator=(const ctkAbstractFactory&); /// Not implemented
-
+  */
   QSharedPointer<HashType> RegisteredItemMap;
 
   bool Verbose;

Libs/Core/ctkAbstractFileBasedFactory.h

@@ -31,12 +31,12 @@
 //----------------------------------------------------------------------------
 /// \ingroup Core
 template<typename BaseClassType>
-class ctkAbstractFactoryFileBasedItem : public ctkAbstractFactoryItem<BaseClassType>
+class ctkAbstractFactoryFileBasedItem
+  : public ctkAbstractFactoryItem<BaseClassType>
 {
 public:
-  explicit ctkAbstractFactoryFileBasedItem(const QString& path);
- 
   /// Get path associated with the object identified by \a key
+  void setPath(const QString& path);
   QString path()const;
 
 private:
@@ -49,23 +49,20 @@ template<typename BaseClassType>
 class ctkAbstractFileBasedFactory : public ctkAbstractFactory<BaseClassType>
 {
 public:
-
-  /// Constructor
-  explicit ctkAbstractFileBasedFactory();
-  virtual ~ctkAbstractFileBasedFactory();
-
   /// Get path associated with the library identified by \a key
   virtual QString path(const QString& key);
 
+  bool registerFileItem(const QFileInfo& file);
+  bool registerFileItem(const QString& key, const QFileInfo& file);
+
 protected:
   void registerAllFileItems(const QStringList& directories);
 
-  virtual ctkAbstractFactoryItem<BaseClassType>* createFactoryFileBasedItem(const QFileInfo& file);
-  virtual QString fileNameToKey(const QString& objectName)const;
+  virtual bool isValidFile(const QFileInfo& file)const;
+  virtual ctkAbstractFactoryItem<BaseClassType>* createFactoryFileBasedItem();
+  virtual void initItem(ctkAbstractFactoryItem<BaseClassType>* item);
 
-private:
-  ctkAbstractFileBasedFactory(const ctkAbstractFileBasedFactory &);  /// Not implemented
-  void operator=(const ctkAbstractFileBasedFactory&); /// Not implemented
+  virtual QString fileNameToKey(const QString& objectName)const;
 };
 
 #include "ctkAbstractFileBasedFactory.tpp"

Libs/Core/ctkAbstractFileBasedFactory.tpp

@@ -32,10 +32,9 @@
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
-ctkAbstractFactoryFileBasedItem<BaseClassType>::ctkAbstractFactoryFileBasedItem(const QString& _path)
-  :ctkAbstractFactoryItem<BaseClassType>()
-  ,Path(_path)
+void ctkAbstractFactoryFileBasedItem<BaseClassType>::setPath(const QString& path)
 {
+  this->Path = path;
 }
 
 //----------------------------------------------------------------------------
@@ -48,19 +47,6 @@ QString ctkAbstractFactoryFileBasedItem<BaseClassType>::path()const
 //----------------------------------------------------------------------------
 // ctkAbstractFileBasedFactory methods
 
-//-----------------------------------------------------------------------------
-template<typename BaseClassType>
-ctkAbstractFileBasedFactory<BaseClassType>::ctkAbstractFileBasedFactory()
-  :ctkAbstractFactory<BaseClassType>()
-{
-}
-
-//-----------------------------------------------------------------------------
-template<typename BaseClassType>
-ctkAbstractFileBasedFactory<BaseClassType>::~ctkAbstractFileBasedFactory()
-{
-}
-
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
 QString ctkAbstractFileBasedFactory<BaseClassType>::path(const QString& key)
@@ -90,7 +76,7 @@ void ctkAbstractFileBasedFactory<BaseClassType>::registerAllFileItems(const QStr
         fileInfo = QFileInfo(fileInfo.symLinkTarget());
         }
       // Skip if item isn't a file
-      if (!fileInfo.isFile())
+      if (!this->isValidFile(fileInfo))
         {
         continue;
         }
@@ -100,38 +86,72 @@ void ctkAbstractFileBasedFactory<BaseClassType>::registerAllFileItems(const QStr
         qDebug() << "Attempt to register command line module:" << fileInfo.fileName();
         }
 
-      QString key = this->fileNameToKey(fileInfo.filePath());
-      QSharedPointer<ctkAbstractFactoryItem<BaseClassType> >
-        itemToRegister = QSharedPointer<ctkAbstractFactoryItem<BaseClassType> >(
-          this->createFactoryFileBasedItem(fileInfo));
-      if (itemToRegister.isNull())
-        {
-        continue;
-        }
-      itemToRegister->setVerbose(this->verbose());
-      if (!this->registerItem(key, itemToRegister))
-        {
-        if (this->verbose())
-          {
-          qWarning() << "Failed to register module: " << key;
-          }
-        continue;
-        }
+      this->registerFileItem(fileInfo);
       }
     }
 }
 
 //-----------------------------------------------------------------------------
 template<typename BaseClassType>
+bool ctkAbstractFileBasedFactory<BaseClassType>
+::registerFileItem(const QFileInfo& fileInfo)
+{
+  QString key = this->fileNameToKey(fileInfo.filePath());
+  return this->registerFileItem(key, fileInfo);
+}
+
+//-----------------------------------------------------------------------------
+template<typename BaseClassType>
+bool ctkAbstractFileBasedFactory<BaseClassType>
+::registerFileItem(const QString& key, const QFileInfo& fileInfo)
+{
+  if (this->item(key))
+    {
+    return false;
+    }
+  QSharedPointer<ctkAbstractFactoryItem<BaseClassType> >
+    itemToRegister(this->createFactoryFileBasedItem());
+  if (itemToRegister.isNull())
+    {
+    return false;
+    }
+  dynamic_cast<ctkAbstractFactoryFileBasedItem<BaseClassType>*>(itemToRegister.data())
+    ->setPath(fileInfo.filePath());
+  this->initItem(itemToRegister.data());
+  bool res = this->registerItem(key, itemToRegister);
+  if (!res && this->verbose())
+    {
+    qWarning() << "Failed to register module: " << key;
+    }
+  return res;
+}
+
+//-----------------------------------------------------------------------------
+template<typename BaseClassType>
+bool ctkAbstractFileBasedFactory<BaseClassType>
+::isValidFile(const QFileInfo& file)const
+{
+  return file.isFile();
+}
+
+//-----------------------------------------------------------------------------
+template<typename BaseClassType>
 ctkAbstractFactoryItem<BaseClassType>* ctkAbstractFileBasedFactory<BaseClassType>
-::createFactoryFileBasedItem(const QFileInfo& file)
+::createFactoryFileBasedItem()
 {
-  Q_UNUSED(file);
   return 0;
 }
 
 //-----------------------------------------------------------------------------
 template<typename BaseClassType>
+void ctkAbstractFileBasedFactory<BaseClassType>::
+initItem(ctkAbstractFactoryItem<BaseClassType>* item)
+{
+  item->setVerbose(this->verbose());
+}
+
+//-----------------------------------------------------------------------------
+template<typename BaseClassType>
 QString ctkAbstractFileBasedFactory<BaseClassType>
 ::fileNameToKey(const QString& fileName)const
 {

Libs/Core/ctkAbstractLibraryFactory.h

@@ -39,7 +39,7 @@ protected:
   typedef typename QHash<QString, void*>::iterator       Iterator;
 
 public:
-  explicit ctkFactoryLibraryItem(const QString& path);
+  //explicit ctkFactoryLibraryItem(const QString& path);
  
   virtual bool load();
   virtual QString loadErrorString()const;
@@ -67,33 +67,18 @@ private:
 //----------------------------------------------------------------------------
 /// \ingroup Core
 template<typename BaseClassType>
-class ctkAbstractLibraryFactory : public ctkAbstractFileBasedFactory<BaseClassType>
+class ctkAbstractLibraryFactory
+  : public ctkAbstractFileBasedFactory<BaseClassType>
 {
 public:
-  /// 
-  /// Constructor
-  explicit ctkAbstractLibraryFactory();
-  virtual ~ctkAbstractLibraryFactory();
-
   /// Set the list of symbols
   void setSymbols(const QStringList& symbols);
-  
-  /// \brief Register a plugin in the factory
-  /// The parameter \a key must be unique
-  bool registerLibrary(const QString& key, const QFileInfo& file);
-
-  /// \brief Utility function to register a QLibrary
-  /// The parameter \a key must be unique
-  bool registerQLibrary(const QString& key, const QFileInfo& file);
 
 protected:
-  virtual ctkFactoryLibraryItem<BaseClassType>* createFactoryLibraryItem(
-    const QFileInfo& library)const;
+  virtual bool isValidFile(const QFileInfo& file)const;
+  virtual void initItem(ctkAbstractFactoryItem<BaseClassType>* item);
 
 private:
-  ctkAbstractLibraryFactory(const ctkAbstractLibraryFactory &);  /// Not implemented
-  void operator=(const ctkAbstractLibraryFactory&); /// Not implemented
-
   QStringList Symbols;
 };
 

Libs/Core/ctkAbstractLibraryFactory.tpp

@@ -26,13 +26,13 @@
 
 //----------------------------------------------------------------------------
 // ctkFactoryLibraryItem methods
-
+/*
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
 ctkFactoryLibraryItem<BaseClassType>::ctkFactoryLibraryItem(const QString& _path)
   :ctkAbstractFactoryFileBasedItem<BaseClassType>(_path)
 {
-}
+}*/
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
@@ -116,67 +116,28 @@ void* ctkFactoryLibraryItem<BaseClassType>::symbolAddress(const QString& symbol)
 
 //-----------------------------------------------------------------------------
 template<typename BaseClassType>
-ctkAbstractLibraryFactory<BaseClassType>::ctkAbstractLibraryFactory()
-  :ctkAbstractFileBasedFactory<BaseClassType>()
-{
-}
-  
-//-----------------------------------------------------------------------------
-template<typename BaseClassType>
-ctkAbstractLibraryFactory<BaseClassType>::~ctkAbstractLibraryFactory()
-{
-}
-
-//-----------------------------------------------------------------------------
-template<typename BaseClassType>
 void ctkAbstractLibraryFactory<BaseClassType>::setSymbols(
-  const QStringList& symbols) 
+  const QStringList& symbols)
 {
-  this->Symbols = symbols; 
-}
-
-//-----------------------------------------------------------------------------
-template<typename BaseClassType>
-bool ctkAbstractLibraryFactory<BaseClassType>::registerLibrary(
-  const QString& key, const QFileInfo& file)
-{
-  QSharedPointer<ctkFactoryLibraryItem<BaseClassType> > itemToRegister =
-    QSharedPointer<ctkFactoryLibraryItem<BaseClassType> >(
-      this->createFactoryLibraryItem(file));
-      //new ctkFactoryLibraryItem<BaseClassType>(key, file.filePath()));
-  if (itemToRegister.isNull())
-    {
-    return false;
-    }
-  itemToRegister->setVerbose(this->verbose());
-  itemToRegister->setSymbols(this->Symbols);
-  return this->registerItem(key, itemToRegister);
+  this->Symbols = symbols;
 }
 
 //-----------------------------------------------------------------------------
 template<typename BaseClassType>
-bool ctkAbstractLibraryFactory<BaseClassType>::registerQLibrary(
-  const QString& key, const QFileInfo& file)
+bool ctkAbstractLibraryFactory<BaseClassType>
+::isValidFile(const QFileInfo& file)const
 {
-  // Skip if current file isn't a library
-  if (!QLibrary::isLibrary(file.fileName()))
-    {
-    return false;
-    }
-  if (this->verbose())
-    {
-    qDebug() << "Attempt to register QLibrary:" << file.fileName();
-    }
-  return this->registerLibrary(key, file);
+  return this->ctkAbstractFileBasedFactory<BaseClassType>::isValidFile(file) &&
+    QLibrary::isLibrary(file.fileName());
 }
 
 //-----------------------------------------------------------------------------
 template<typename BaseClassType>
-ctkFactoryLibraryItem<BaseClassType>* ctkAbstractLibraryFactory<BaseClassType>::
-createFactoryLibraryItem(const QFileInfo& library)const
+void ctkAbstractLibraryFactory<BaseClassType>::
+initItem(ctkAbstractFactoryItem<BaseClassType>* item)
 {
-  Q_UNUSED(library);
-  return 0;
+  this->ctkAbstractFileBasedFactory<BaseClassType>::initItem(item);
+  dynamic_cast<ctkFactoryLibraryItem<BaseClassType>*>(item)->setSymbols(this->Symbols);
 }
 
 #endif

Libs/Core/ctkAbstractPluginFactory.h

@@ -34,7 +34,6 @@ template<typename BaseClassType>
 class ctkFactoryPluginItem : public ctkAbstractFactoryFileBasedItem<BaseClassType>
 {
 public:
-  explicit ctkFactoryPluginItem(const QString& path);
   virtual bool load();
   virtual QString loadErrorString()const;
 
@@ -50,19 +49,9 @@ private:
 template<typename BaseClassType>
 class ctkAbstractPluginFactory : public ctkAbstractFileBasedFactory<BaseClassType>
 {
-public:
-  /// Constructor
-  explicit ctkAbstractPluginFactory();
-
-  /// Register a plugin in the factory
-  virtual bool registerLibrary(const QString& key, const QFileInfo& file);
-
 protected:
-  virtual ctkAbstractFactoryItem<BaseClassType>* createFactoryPluginItem(const QFileInfo& file);
-
-private:
-  ctkAbstractPluginFactory(const ctkAbstractPluginFactory &);  /// Not implemented
-  void operator=(const ctkAbstractPluginFactory&); /// Not implemented
+  virtual bool isValidFile(const QFileInfo& file)const;
+  virtual ctkAbstractFactoryItem<BaseClassType>* createFactoryFileBasedItem();
 };
 
 #include "ctkAbstractPluginFactory.tpp"

Libs/Core/ctkAbstractPluginFactory.tpp

@@ -30,13 +30,14 @@
 
 //----------------------------------------------------------------------------
 // ctkFactoryPluginItem methods
-
+/*
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
 ctkFactoryPluginItem<BaseClassType>::ctkFactoryPluginItem(const QString& _path)
   :ctkAbstractFactoryFileBasedItem<BaseClassType>(_path)
 {
 }
+*/
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
@@ -84,39 +85,21 @@ BaseClassType* ctkFactoryPluginItem<BaseClassType>::instanciator()
 //----------------------------------------------------------------------------
 // ctkAbstractPluginFactory methods
 
-//----------------------------------------------------------------------------
-template<typename BaseClassType>
-ctkAbstractPluginFactory<BaseClassType>::ctkAbstractPluginFactory()
-:ctkAbstractFileBasedFactory<BaseClassType>()
-{
-}
-
-//----------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
 template<typename BaseClassType>
-bool ctkAbstractPluginFactory<BaseClassType>::registerLibrary(const QString& key, const QFileInfo& file)
+ctkAbstractFactoryItem<BaseClassType>* ctkAbstractPluginFactory<BaseClassType>
+::createFactoryFileBasedItem()
 {
-  // Check if already registered
-  if (this->item(key))
-    {
-    return false;
-    }
-  QSharedPointer<ctkAbstractFactoryItem<BaseClassType> > _item =
-    QSharedPointer<ctkAbstractFactoryItem<BaseClassType> >(
-      this->createFactoryPluginItem(file));
-  if (_item.isNull())
-    {
-    return false;
-    }
-  
-  _item->setVerbose(this->verbose());
-  return this->registerItem(key, _item);
+  return new ctkFactoryPluginItem<BaseClassType>();
 }
 
-//----------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
 template<typename BaseClassType>
-ctkAbstractFactoryItem<BaseClassType>* ctkAbstractPluginFactory<BaseClassType>::createFactoryPluginItem(const QFileInfo& file)
+bool ctkAbstractPluginFactory<BaseClassType>
+::isValidFile(const QFileInfo& fileInfo)const
 {
-  return new ctkFactoryPluginItem<BaseClassType>(file.filePath());
+  return this->ctkAbstractFileBasedFactory<BaseClassType>::isValidFile(fileInfo) &&
+    QLibrary::isLibrary(fileInfo.fileName());
 }
 
 #endif