From e07d63a9117bb75abf0da4da57d3aee418e03514 Mon Sep 17 00:00:00 2001
From: Martyn Gigg <martyn.gigg@stfc.ac.uk>
Date: Mon, 16 Jul 2018 11:15:40 +0100
Subject: [PATCH] Remove temp ADS entry if exception is thrown

Also adds longer comment to explain the reason behind
doing this behaviour.
Refs #22927
---
 .../api/src/Exports/BinaryOperations.cpp      | 49 ++++++++++++-------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/Framework/PythonInterface/mantid/api/src/Exports/BinaryOperations.cpp b/Framework/PythonInterface/mantid/api/src/Exports/BinaryOperations.cpp
index 7d5afdeec3a..e35e23bf6a8 100644
--- a/Framework/PythonInterface/mantid/api/src/Exports/BinaryOperations.cpp
+++ b/Framework/PythonInterface/mantid/api/src/Exports/BinaryOperations.cpp
@@ -156,39 +156,50 @@ ResultType performBinaryOpWithDouble(const LHSType inputWS, const double value,
                                      const std::string &op,
                                      const std::string &name, bool inplace,
                                      bool reverse) {
-  const std::string &algoName = op;
-
-  // Create the single valued workspace first so that it is run as a top-level
-  // algorithm
-  // such that it's history can be recreated
-  API::Algorithm_sptr alg = API::AlgorithmManager::Instance().createUnmanaged(
+  // RAII struct to add/remove workspace from ADS
+  struct ScopedADSEntry {
+    ScopedADSEntry(const std::string &entryName,
+                   const MatrixWorkspace_sptr &value)
+        : name(entryName) {
+      ads.addOrReplace(entryName, value);
+    }
+    ~ScopedADSEntry() { ads.remove(name); }
+
+    const std::string &name;
+    API::AnalysisDataServiceImpl &ads = API::AnalysisDataService::Instance();
+  };
+
+  // In order to recreate a history record of the final binary operation
+  // there must be a record of the creation of the single value workspace used
+  // on the RHS here. This is achieved by running CreateSingleValuedWorkspace
+  // algorithm and adding the output workspace to the ADS. Adding the output
+  // to the ADS is critical so that workspace.name() is updated, by the ADS, to
+  // return the same string. WorkspaceProperty<TYPE>::createHistory() then
+  // records the correct workspace name for input into the final binary
+  // operation rather than creating a temporary name.
+  auto alg = API::AlgorithmManager::Instance().createUnmanaged(
       "CreateSingleValuedWorkspace");
   alg->setChild(false);
+  // we manually store the workspace as it's easier to retrieve the correct
+  // type from alg->getProperty rather than calling the ADS again and casting
   alg->setAlwaysStoreInADS(false);
   alg->initialize();
   alg->setProperty<double>("DataValue", value);
-  const std::string tmp_name("__single_value");
-  alg->setPropertyValue("OutputWorkspace", tmp_name);
+  const std::string tmpName("__python_binary_op_single_value");
+  alg->setPropertyValue("OutputWorkspace", tmpName);
   alg->execute();
 
-  auto &ads = Mantid::API::AnalysisDataService::Instance();
   MatrixWorkspace_sptr singleValue;
   if (alg->isExecuted()) {
     singleValue = alg->getProperty("OutputWorkspace");
-
-    // We must store this in the ADS to force the workspace
-    // to have a name, so that the history entry does not
-    // use a temporary name which changes from time to time
-    ads.add(tmp_name, singleValue);
   } else {
-    throw std::runtime_error(
-        "performBinaryOp: Error in execution of CreateSingleValuedWorkspace");
+    throw std::runtime_error("performBinaryOp: Error in execution of "
+                             "CreateSingleValuedWorkspace");
   }
-  // Call the function above with the single-value workspace
+  ScopedADSEntry removeOnExit(tmpName, singleValue);
   ResultType result =
       performBinaryOp<LHSType, MatrixWorkspace_sptr, ResultType>(
-          inputWS, singleValue, algoName, name, inplace, reverse);
-  ads.remove(tmp_name);
+          inputWS, singleValue, op, name, inplace, reverse);
   return result;
 }
 
-- 
GitLab