Factory - Update API (Shared items, msg display)

API:

 * Function "ctkAbstractFactory::keys()" has been changed into "ctkAbstractFactory::itemKeys()"



Update related to "sharedItems" API:

 * Before this change the map shared between the factory was also used
to instantiate item for each factory. That means that each factory
was iterating over all modules registered by all factories sharing the
same map. This approach was sub optimal.

 * Following this change, the map of items registered in each factory is
now different from the map allowing to keep track of the items registered
by all the factories having common map of shared items. This allow a given
factory to loop over and instantiate only the items associated to it.



Message display:

 * The display of registration message has also been centralized under
a single function "displayRegistrationStatus" allowing to nicely format it.

Libs/Core/Testing/Cpp/ctkAbstractFactoryTest1.cpp

@@ -80,7 +80,7 @@ int ctkAbstractFactoryTest1(int argc, char * argv [] )
     return EXIT_FAILURE;
     }
   abstractFactory.registerItems();
-  if (abstractFactory.keys().count() != 0)
+  if (abstractFactory.itemKeys().count() != 0)
     {
     std::cerr<< "ctkAbstractFactory::keys() failed" << std::endl;
     return EXIT_FAILURE;
@@ -108,9 +108,9 @@ int ctkAbstractFactoryTest1(int argc, char * argv [] )
     return EXIT_FAILURE;
     }
   factory.registerItems();
-  if (factory.keys().count() != 2 ||
-      factory.keys()[0] != "item1" || 
-      factory.keys()[1] != "item2")
+  if (factory.itemKeys().count() != 2 ||
+      factory.itemKeys()[0] != "item1" ||
+      factory.itemKeys()[1] != "item2")
     {
     std::cerr<< "ctkAbstractFactory::keys() failed" << std::endl;
     return EXIT_FAILURE;

Libs/Core/Testing/Cpp/ctkAbstractLibraryFactoryTest1.cpp

@@ -104,18 +104,18 @@ int ctkAbstractLibraryFactoryTest1(int argc, char * argv [])
     }
   
   res = libraryFactory.registerFileItem("lib", file);
-  if (!res || libraryFactory.keys().count() != 1)
+  if (!res || libraryFactory.itemKeys().count() != 1)
     {
     std::cerr << "ctkAbstractLibraryFactory::registerLibrary() failed"
-              << libraryFactory.keys().count() << std::endl;
+              << libraryFactory.itemKeys().count() << std::endl;
     return EXIT_FAILURE;
     }
   // register twice must return false
   res = libraryFactory.registerFileItem("lib", file);
-  if (res || libraryFactory.keys().count() != 1)
+  if (res || libraryFactory.itemKeys().count() != 1)
     {
     std::cerr << "ctkAbstractLibraryFactory::registerLibrary() failed"
-              << libraryFactory.keys().count() << std::endl;
+              << libraryFactory.itemKeys().count() << std::endl;
     return EXIT_FAILURE;
     }
   if (QFileInfo(libraryFactory.path("lib")) != file)

Libs/Core/Testing/Cpp/ctkAbstractPluginFactoryTest1.cpp

@@ -72,18 +72,18 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
     }
   
   res = pluginFactory.registerFileItem("lib", file);
-  if (!res || pluginFactory.keys().count() != 1)
+  if (!res || pluginFactory.itemKeys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed"
-              << pluginFactory.keys().count() << std::endl;
+              << pluginFactory.itemKeys().count() << std::endl;
     return EXIT_FAILURE;
     }
   // register twice must return false
   res = pluginFactory.registerFileItem("lib", file);
-  if (res || pluginFactory.keys().count() != 1)
+  if (res || pluginFactory.itemKeys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed"
-              << pluginFactory.keys().count() << std::endl;
+              << pluginFactory.itemKeys().count() << std::endl;
     return EXIT_FAILURE;
     }
   if (QFileInfo(pluginFactory.path("lib")) != file)
@@ -107,7 +107,7 @@ int ctkAbstractPluginFactoryTest1(int argc, char * argv [])
   buttonPluginFactory.setVerbose(true);
   // it should register but fail while instanciating
   res = buttonPluginFactory.registerFileItem("foo", file);
-  if (!res || buttonPluginFactory.keys().count() != 1)
+  if (!res || buttonPluginFactory.itemKeys().count() != 1)
     {
     std::cerr << "ctkAbstractPluginFactory::registerLibrary() failed" << std::endl;
     return EXIT_FAILURE;

Libs/Core/ctkAbstractFactory.h

@@ -44,14 +44,14 @@ class ctkAbstractFactoryItem
 public:
   //explicit ctkAbstractFactoryItem();
   ctkAbstractFactoryItem();
-  
+
   virtual QString loadErrorString()const;
   virtual bool load() = 0;
-  
+
   BaseClassType* instantiate();
   bool instantiated()const;
   virtual void uninstantiate();
-  
+
   void setVerbose(bool value);
   bool verbose()const;
 
@@ -76,6 +76,7 @@ template<typename BaseClassType>
 class ctkAbstractFactory
 {
 public:
+
   typedef QHash<QString, QSharedPointer<ctkAbstractFactoryItem<BaseClassType> > > HashType;
 
   /// Constructor/Desctructor
@@ -95,8 +96,11 @@ public:
   /// Should be overloaded in subclasse
   virtual QString path(const QString& itemKey){ Q_UNUSED(itemKey); return QString(); }
 
+  void setSharedItems(const QSharedPointer<HashType>& items);
+  QSharedPointer<HashType> sharedItems();
+
   /// Get list of all registered item keys.
-  QStringList keys() const;
+  QStringList itemKeys() const;
 
   /// \brief Register items with the factory
   /// Method provided for convenience - Should be overloaded in subclasse
@@ -107,11 +111,11 @@ public:
   void setVerbose(bool value);
   bool verbose()const;
 
-  void setRegisteredItems(const QSharedPointer<HashType>& items);
-  QSharedPointer<HashType> registeredItems();
-
 protected:
 
+  void displayRegistrationStatus(QtMsgType type, const QString& description,
+                                 const QString& status, bool display);
+
   /// \brief Call the load method associated with the item.
   /// If succesfully loaded, add it to the internal map.
   bool registerItem(const QString& key, const QSharedPointer<ctkAbstractFactoryItem<BaseClassType> > & item);
@@ -119,6 +123,8 @@ protected:
   /// Get a Factory item given its itemKey. Return 0 if any.
   ctkAbstractFactoryItem<BaseClassType> * item(const QString& itemKey)const;
 
+  ctkAbstractFactoryItem<BaseClassType> * sharedItem(const QString& itemKey)const;
+
   typedef typename HashType::const_iterator ConstIterator;
   typedef typename HashType::iterator       Iterator;
 
@@ -127,7 +133,9 @@ private:
   ctkAbstractFactory(const ctkAbstractFactory &); /// Not implemented
   void operator=(const ctkAbstractFactory&); /// Not implemented
   */
-  QSharedPointer<HashType> RegisteredItemMap;
+  HashType RegisteredItemMap;
+  QSharedPointer<HashType> SharedRegisteredItemMap;
+
 
   bool Verbose;
 };

Libs/Core/ctkAbstractFactory.tpp

@@ -100,7 +100,7 @@ template<typename BaseClassType>
 ctkAbstractFactory<BaseClassType>::ctkAbstractFactory()
 {
   this->Verbose = false;
-  this->RegisteredItemMap = QSharedPointer<HashType>(new HashType);
+  this->SharedRegisteredItemMap = QSharedPointer<HashType>(new HashType);
 }
 
 //----------------------------------------------------------------------------
@@ -139,11 +139,52 @@ void ctkAbstractFactory<BaseClassType>::uninstantiate(const QString& itemKey)
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
-QStringList ctkAbstractFactory<BaseClassType>::keys() const
+void ctkAbstractFactory<BaseClassType>::setSharedItems(const QSharedPointer<HashType>& items)
+{
+  this->SharedRegisteredItemMap = items;
+}
+
+//----------------------------------------------------------------------------
+template<typename BaseClassType>
+QSharedPointer<typename ctkAbstractFactory<BaseClassType>::HashType>
+ctkAbstractFactory<BaseClassType>::sharedItems()
+{
+  return this->SharedRegisteredItemMap;
+}
+
+//----------------------------------------------------------------------------
+template<typename BaseClassType>
+QStringList ctkAbstractFactory<BaseClassType>::itemKeys() const
 {
   // Since by construction, we checked if a name was already in the QHash,
   // there is no need to call 'uniqueKeys'
-  return this->RegisteredItemMap->keys();
+  return this->RegisteredItemMap.keys();
+}
+
+//----------------------------------------------------------------------------
+template<typename BaseClassType>
+void ctkAbstractFactory<BaseClassType>::displayRegistrationStatus(
+    QtMsgType type, const QString& description, const QString& status, bool display)
+{
+  QString msg = QString("%1 [%2]").arg(description + " ", -70, QChar('.')).arg(status);
+  if (display)
+    {
+    switch(type)
+      {
+      case QtFatalMsg:
+        qFatal("%s", qPrintable(msg));
+        break;
+      case QtCriticalMsg:
+        qCritical("%s", qPrintable(msg));
+        break;
+      case QtWarningMsg:
+        qWarning("%s", qPrintable(msg));
+        break;
+      case QtDebugMsg:
+        qDebug("%s", qPrintable(msg));
+        break;
+      }
+    }
 }
 
 //----------------------------------------------------------------------------
@@ -161,6 +202,15 @@ bool ctkAbstractFactory<BaseClassType>::registerItem(const QString& key,
       }
     return false;
     }
+
+  if (this->sharedItem(key))
+    {
+    if (this->verbose())
+      {
+      qDebug() << "Item" << key << "has already been registered";
+      }
+    return false;
+    }
   
   // Attempt to load it
   if (!_item->load())
@@ -177,8 +227,10 @@ bool ctkAbstractFactory<BaseClassType>::registerItem(const QString& key,
     return false;
     }
   
-  // Store its reference using a QSharedPointer
-  this->RegisteredItemMap->insert(key, _item);
+  // Store item reference using a QSharedPointer
+  this->RegisteredItemMap.insert(key, _item);
+  this->SharedRegisteredItemMap.data()->insert(key, _item);
+
   return true;
 }
 
@@ -186,8 +238,8 @@ bool ctkAbstractFactory<BaseClassType>::registerItem(const QString& key,
 template<typename BaseClassType>
 ctkAbstractFactoryItem<BaseClassType> * ctkAbstractFactory<BaseClassType>::item(const QString& itemKey)const
 {
-  ConstIterator iter = this->RegisteredItemMap->find(itemKey);
-  if ( iter == this->RegisteredItemMap->constEnd())
+  ConstIterator iter = this->RegisteredItemMap.find(itemKey);
+  if ( iter == this->RegisteredItemMap.constEnd())
     {
     return 0;
     }
@@ -196,31 +248,32 @@ ctkAbstractFactoryItem<BaseClassType> * ctkAbstractFactory<BaseClassType>::item(
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
-void ctkAbstractFactory<BaseClassType>::setVerbose(bool value)
-{
-  this->Verbose = value;
-}
-
-//----------------------------------------------------------------------------
-template<typename BaseClassType>
-bool ctkAbstractFactory<BaseClassType>::verbose()const
+ctkAbstractFactoryItem<BaseClassType> * ctkAbstractFactory<BaseClassType>::sharedItem(const QString& itemKey)const
 {
-  return this->Verbose;
+  if(this->SharedRegisteredItemMap.isNull())
+    {
+    return 0;
+    }
+  ConstIterator iter = this->SharedRegisteredItemMap.data()->find(itemKey);
+  if ( iter == this->SharedRegisteredItemMap.data()->constEnd())
+    {
+    return 0;
+    }
+  return iter.value().data();
 }
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
-void ctkAbstractFactory<BaseClassType>::setRegisteredItems(const QSharedPointer<HashType>& items)
+void ctkAbstractFactory<BaseClassType>::setVerbose(bool value)
 {
-  this->RegisteredItemMap = items;
+  this->Verbose = value;
 }
 
 //----------------------------------------------------------------------------
 template<typename BaseClassType>
-QSharedPointer<typename ctkAbstractFactory<BaseClassType>::HashType>
-ctkAbstractFactory<BaseClassType>::registeredItems()
+bool ctkAbstractFactory<BaseClassType>::verbose()const
 {
-  return this->RegisteredItemMap;
+  return this->Verbose;
 }
 
 #endif

Libs/Core/ctkAbstractFileBasedFactory.tpp

@@ -99,27 +99,32 @@ template<typename BaseClassType>
 bool ctkAbstractFileBasedFactory<BaseClassType>
 ::registerFileItem(const QString& key, const QFileInfo& fileInfo)
 {
-  if (this->verbose())
-    {
-    qDebug() << "Attempt to register:" << fileInfo.fileName();
-    }
-  if (this->item(key))
+  QString description = QString("Attempt to register \"%1\"").arg(key);
+  if(this->sharedItem(key) || this->item(key))
     {
+    this->displayRegistrationStatus(QtDebugMsg, description,
+                                    "Already registered", this->verbose());
     return false;
     }
   QSharedPointer<ctkAbstractFactoryItem<BaseClassType> >
     itemToRegister(this->createFactoryFileBasedItem());
   if (itemToRegister.isNull())
     {
+    this->displayRegistrationStatus(QtWarningMsg, description,
+                                    "Failed to create FileBasedItem", this->verbose());
     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())
+  if(res)
+    {
+    this->displayRegistrationStatus(QtDebugMsg, description, "OK", this->verbose());
+    }
+  else
     {
-    qWarning() << "Failed to register module: " << key;
+    this->displayRegistrationStatus(QtWarningMsg, description, "Failed", this->verbose());
     }
   return res;
 }

Libs/Core/ctkAbstractObjectFactory.tpp

@@ -60,20 +60,27 @@ template<typename BaseClassType>
 template<typename ClassType>
 bool ctkAbstractObjectFactory<BaseClassType>::registerObject(const QString& key)
 {
-  if (this->verbose())
-    {
-    qDebug() << "Attempt to register:" << key;
-    }
-  // Check if already registered
-  if (this->item(key))
+  QString description = QString("Attempt to register \"%1\"").arg(key);
+  if(this->sharedItem(key) || this->item(key))
     {
+    this->displayRegistrationStatus(QtDebugMsg, description,
+                                    "Already registered", this->verbose());
     return false;
     }
   QSharedPointer<ctkFactoryObjectItem<BaseClassType, ClassType> > objectItem =
     QSharedPointer<ctkFactoryObjectItem<BaseClassType, ClassType> >(
       new ctkFactoryObjectItem<BaseClassType, ClassType>() );
   objectItem->setVerbose(this->verbose());
-  return this->registerItem(key, objectItem);
+  bool res = this->registerItem(key, objectItem);
+  if(res)
+    {
+    this->displayRegistrationStatus(QtDebugMsg, description, "OK", this->verbose());
+    }
+  else
+    {
+    this->displayRegistrationStatus(QtWarningMsg, description, "Failed", this->verbose());
+    }
+  return res;
 }
 
 #endif