Add ability to replace instantiators to DynamicFactory rather than

just on the AlgorithmFactory. Refs #970

Code/Mantid/Framework/API/inc/MantidAPI/AlgorithmFactory.h

@@ -77,10 +77,10 @@ class MANTID_API_DLL AlgorithmFactoryImpl : public Kernel::DynamicFactory<Algori
    * Subscribes an algorithm using a custom instantiator. This
    * object takes ownership of the instantiator
    * @param instantiator - A pointer to a custom instantiator
-   * @param replaceExisting - If true an existing algorithm of the same name and version is replaced
+   * @param replaceExisting - Defines what happens if an algorithm of the same name/verson already exists, see SubscribeAction
    */
   template<class T>
-  void subscribe(Kernel::AbstractInstantiator<T> *instantiator, const bool replaceExisting = false)
+  void subscribe(Kernel::AbstractInstantiator<T> *instantiator, const SubscribeAction replaceExisting = ErrorIfExists)
   {
     boost::shared_ptr<IAlgorithm> tempAlg = instantiator-> createInstance();
     const int version = extractAlgVersion(tempAlg);
@@ -95,11 +95,7 @@ class MANTID_API_DLL AlgorithmFactoryImpl : public Kernel::DynamicFactory<Algori
       }
       else
       {
-        if( replaceExisting )
-        {
-          Kernel::DynamicFactory<Algorithm>::unsubscribe(key);
-        }
-        else if(version == it->second)
+        if(version == it->second && replaceExisting == ErrorIfExists)
         {
           g_log.fatal() << "Cannot register algorithm " << className << " twice with the same version\n";
           return;
@@ -109,7 +105,7 @@ class MANTID_API_DLL AlgorithmFactoryImpl : public Kernel::DynamicFactory<Algori
           m_vmap[className]=version;
         }
       }  
-      Kernel::DynamicFactory<Algorithm>::subscribe(key, instantiator);
+      Kernel::DynamicFactory<Algorithm>::subscribe(key, instantiator, replaceExisting);
     }
   }
   /// Unsubscribe the given algorithm

Code/Mantid/Framework/Kernel/inc/MantidKernel/DynamicFactory.h

@@ -63,8 +63,12 @@ namespace Kernel
 template <class Base>
 class DynamicFactory
 {
+
+public:
   /// Defines the whether notifications are dispatched
   enum NotificationStatus {Enabled, Disabled};
+  /// Defines replacement behaviour
+  enum SubscribeAction {ErrorIfExists, OverwriteCurrent};
 
 public:
   /**
@@ -162,10 +166,15 @@ class DynamicFactory
   /// and the instantiator is deleted.
   /// @param className :: the name of the class you wish to subscribe
   /// @param pAbstractFactory :: A pointer to an abstractFactory for this class
-  void subscribe(const std::string& className, AbstractFactory* pAbstractFactory)
+  /// @param replace :: If ReplaceExisting then the given AbstractFactory replaces any existing
+  ///                   factory with the same className, else throws std::runtime_error (default=ThrowOnExisting)
+  void subscribe(const std::string& className, AbstractFactory* pAbstractFactory,
+                 SubscribeAction replace=ErrorIfExists)
   {
+    if(className.empty()) throw std::invalid_argument("Cannot register empty class name");
+
     typename FactoryMap::iterator it = _map.find(className);
-    if (!className.empty() && it == _map.end())
+    if (it == _map.end() || replace == OverwriteCurrent)
     {
       _map[className] = pAbstractFactory;
       sendUpdateNotificationIfEnabled();

Code/Mantid/Framework/Kernel/test/DynamicFactoryTest.h

@@ -14,7 +14,7 @@
 using namespace Mantid::Kernel;
 
 // Helper class
-class AFactory : public DynamicFactory<int>
+class IntFactory : public DynamicFactory<int>
 {
 };
 
@@ -43,24 +43,45 @@ class DynamicFactoryTest : public CxxTest::TestSuite
     TS_ASSERT_THROWS( factory.create("testEntry"), std::runtime_error )
     factory.subscribe<int>("testEntry");
     TS_ASSERT_THROWS_NOTHING( int_ptr i = factory.create("testEntry") );
+    factory.unsubscribe("testEntry");
   }
 
   void testCreateUnwrapped()
   {
     TS_ASSERT_THROWS( factory.createUnwrapped("testUnrappedEntry"), std::runtime_error )
     factory.subscribe<int>("testUnwrappedEntry");
     int *i = NULL;
-    TS_ASSERT_THROWS_NOTHING( i = factory.createUnwrapped("testEntry") );
+    TS_ASSERT_THROWS_NOTHING( i = factory.createUnwrapped("testUnwrappedEntry") );
     delete i;
+    factory.unsubscribe("testUnwrappedEntry");
+  }
+
+  void testSubscribeWithEmptyNameThrowsInvalidArgument()
+  {
+    TS_ASSERT_THROWS( factory.subscribe<int>(""), std::invalid_argument);
+  }
+
+  void testSubscribeWithReplaceEqualsErrorIfExistsThrowsRegisteringMatchingClass()
+  {
+    TS_ASSERT_THROWS_NOTHING( factory.subscribe("int",new Instantiator<int, int>));
+    TS_ASSERT_THROWS( factory.subscribe("int",new Instantiator<int, int>,IntFactory::ErrorIfExists), std::runtime_error);
+    factory.unsubscribe("int");
+  }
+
+  void testSubscribeWithReplaceEqualsOverwriteCurrentReplacesMatchingClass()
+  {
+    TS_ASSERT_THROWS_NOTHING( factory.subscribe("int",new Instantiator<int, int>));
+    TS_ASSERT_THROWS_NOTHING( factory.subscribe("int",new Instantiator<int, int>,IntFactory::OverwriteCurrent));
+
+    factory.unsubscribe("int");
   }
 
   void testSubscribeByDefaultDoesNotNotify()
   {
     m_updateNoticeReceived = false;
     TS_ASSERT_THROWS_NOTHING( factory.subscribe<int>("int") );
     TS_ASSERT_EQUALS( m_updateNoticeReceived, false )
-    TS_ASSERT_THROWS_NOTHING( factory.subscribe("int2",new Instantiator<int, int>));
-    TS_ASSERT_THROWS( factory.subscribe<int>("int"), std::runtime_error);
+    factory.unsubscribe("int");
   }
 
   void testSubscribeNotifiesIfTheyAreSwitchedOn()
@@ -94,6 +115,7 @@ class DynamicFactoryTest : public CxxTest::TestSuite
     m_updateNoticeReceived = false;
   }
 
+
   void testExists()
   {
     TS_ASSERT( ! factory.exists("testing") );
@@ -108,13 +130,13 @@ class DynamicFactoryTest : public CxxTest::TestSuite
   }
 
 private:
-  void handleFactoryUpdate(const Poco::AutoPtr<AFactory::UpdateNotification> &)
+  void handleFactoryUpdate(const Poco::AutoPtr<IntFactory::UpdateNotification> &)
   {
     m_updateNoticeReceived = true;
   }
 
-  AFactory factory;
-  Poco::NObserver<DynamicFactoryTest, AFactory::UpdateNotification> m_notificationObserver;
+  IntFactory factory;
+  Poco::NObserver<DynamicFactoryTest, IntFactory::UpdateNotification> m_notificationObserver;
   bool m_updateNoticeReceived;
 };
 

Code/Mantid/Framework/PythonAPI/src/FrameworkManagerProxy.cpp

@@ -549,6 +549,7 @@ void FrameworkManagerProxy::registerPyAlgorithm(boost::python::object pyobj)
   using Mantid::PythonAPI::PythonObjectInstantiator;
   using Mantid::API::Algorithm;
   using Mantid::API::AlgorithmFactory;
+  using Mantid::API::AlgorithmFactoryImpl;
   using boost::python::object;
   using boost::python::handle;
   using boost::python::borrowed;
@@ -559,7 +560,7 @@ void FrameworkManagerProxy::registerPyAlgorithm(boost::python::object pyobj)
   PyObject *classObject = PyObject_GetAttrString(pyobj.ptr(), "__class__");
   object classType(handle<>(borrowed(classObject)));
   // Takes ownership of instantiator and replaces any existing algorithm
-  AlgorithmFactory::Instance().subscribe(new PythonObjectInstantiator<Algorithm>(classType), true);
+  AlgorithmFactory::Instance().subscribe(new PythonObjectInstantiator<Algorithm>(classType), AlgorithmFactoryImpl::OverwriteCurrent);
 }
 
 //--------------------------------------------------------------------------

Code/Mantid/Framework/PythonInterface/mantid/api/src/Exports/AlgorithmFactory.cpp

@@ -109,7 +109,7 @@ namespace
     }
     boost::python::object classType(handle<>(borrowed(classObject)));
     // Takes ownership of instantiator and replaces any existing algorithm
-    AlgorithmFactory::Instance().subscribe(new PythonObjectInstantiator<Algorithm>(classType), true);
+    AlgorithmFactory::Instance().subscribe(new PythonObjectInstantiator<Algorithm>(classType), AlgorithmFactoryImpl::OverwriteCurrent);
   }
 }