From c490b05dbb405ba72e9f45a216af5f210efd990c Mon Sep 17 00:00:00 2001
From: Martyn Gigg <martyn.gigg@gmail.com>
Date: Tue, 8 Oct 2019 16:35:16 +0100
Subject: [PATCH] Define all ReflectionConditions once

Repeatedly creating the vector is wasted work
Refs #26690
---
 .../Crystal/ReflectionCondition.h             |  5 +-
 .../Geometry/src/Crystal/CrystalStructure.cpp |  3 +-
 .../src/Crystal/ReflectionCondition.cpp       | 55 +++++++++++--------
 .../Geometry/test/ReflectionConditionTest.h   | 32 +++++------
 4 files changed, 52 insertions(+), 43 deletions(-)

diff --git a/Framework/Geometry/inc/MantidGeometry/Crystal/ReflectionCondition.h b/Framework/Geometry/inc/MantidGeometry/Crystal/ReflectionCondition.h
index 8e21ed69fcb..fc8876dbef6 100644
--- a/Framework/Geometry/inc/MantidGeometry/Crystal/ReflectionCondition.h
+++ b/Framework/Geometry/inc/MantidGeometry/Crystal/ReflectionCondition.h
@@ -175,9 +175,10 @@ public:
 
 /// Shared pointer to a ReflectionCondition
 using ReflectionCondition_sptr = boost::shared_ptr<ReflectionCondition>;
+/// A collection of reflections
+using ReflectionConditions = std::vector<ReflectionCondition_sptr>;
 
-MANTID_GEOMETRY_DLL std::vector<ReflectionCondition_sptr>
-getAllReflectionConditions();
+MANTID_GEOMETRY_DLL const ReflectionConditions &getAllReflectionConditions();
 MANTID_GEOMETRY_DLL std::vector<std::string> getAllReflectionConditionNames();
 MANTID_GEOMETRY_DLL std::vector<std::string> getAllReflectionConditionSymbols();
 MANTID_GEOMETRY_DLL ReflectionCondition_sptr
diff --git a/Framework/Geometry/src/Crystal/CrystalStructure.cpp b/Framework/Geometry/src/Crystal/CrystalStructure.cpp
index 9bc6c21a626..7f2deee0a7b 100644
--- a/Framework/Geometry/src/Crystal/CrystalStructure.cpp
+++ b/Framework/Geometry/src/Crystal/CrystalStructure.cpp
@@ -114,8 +114,7 @@ void CrystalStructure::setReflectionConditionFromSpaceGroup(
     centering = "Robv";
   }
 
-  std::vector<ReflectionCondition_sptr> reflectionConditions =
-      getAllReflectionConditions();
+  const auto &reflectionConditions = getAllReflectionConditions();
   for (auto &reflectionCondition : reflectionConditions) {
     if (reflectionCondition->getSymbol() == centering) {
       m_centering = reflectionCondition;
diff --git a/Framework/Geometry/src/Crystal/ReflectionCondition.cpp b/Framework/Geometry/src/Crystal/ReflectionCondition.cpp
index 63df6ca89f5..c8f087f5126 100644
--- a/Framework/Geometry/src/Crystal/ReflectionCondition.cpp
+++ b/Framework/Geometry/src/Crystal/ReflectionCondition.cpp
@@ -5,41 +5,52 @@
 //     & Institut Laue - Langevin
 // SPDX - License - Identifier: GPL - 3.0 +
 #include "MantidGeometry/Crystal/ReflectionCondition.h"
-#include "MantidKernel/System.h"
 #include <algorithm>
 #include <iterator>
 
+#include <boost/make_shared.hpp>
+
 namespace Mantid {
 namespace Geometry {
 
+// General template definition for RegisterConditions
+template <typename... Args> struct RegisterConditions;
+
+// Specialisation for recursive case
+template <typename R, typename... Args> struct RegisterConditions<R, Args...> {
+  static void run(ReflectionConditions &container) {
+    container.emplace_back(boost::make_shared<R>());
+    RegisterConditions<Args...>::run(container);
+  }
+};
+
+// Specialisation to end recursion
+template <> struct RegisterConditions<> {
+  static void run(ReflectionConditions &) {}
+};
+
 /** @return a vector with all possible ReflectionCondition objects */
-std::vector<ReflectionCondition_sptr> getAllReflectionConditions() {
-  std::vector<ReflectionCondition_sptr> out;
-  out.push_back(ReflectionCondition_sptr(new ReflectionConditionPrimitive()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionCFaceCentred()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionAFaceCentred()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionBFaceCentred()));
-  out.push_back(ReflectionCondition_sptr(new ReflectionConditionBodyCentred()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionAllFaceCentred()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionRhombohedrallyObverse()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionRhombohedrallyReverse()));
-  out.push_back(
-      ReflectionCondition_sptr(new ReflectionConditionHexagonallyReverse()));
-  return out;
+const ReflectionConditions &getAllReflectionConditions() {
+  static ReflectionConditions conditions;
+  if (conditions.empty()) {
+    RegisterConditions<
+        ReflectionConditionPrimitive, ReflectionConditionCFaceCentred,
+        ReflectionConditionAFaceCentred, ReflectionConditionBFaceCentred,
+        ReflectionConditionBodyCentred, ReflectionConditionAllFaceCentred,
+        ReflectionConditionRhombohedrallyObverse,
+        ReflectionConditionRhombohedrallyReverse,
+        ReflectionConditionHexagonallyReverse>::run(conditions);
+  }
+  return conditions;
 }
 
 /// Helper function that transforms all ReflectionConditions to strings.
 std::vector<std::string> transformReflectionConditions(
     const std::function<std::string(const ReflectionCondition_sptr &)> &fn) {
-  auto conditions = getAllReflectionConditions();
+  const auto &conditions = getAllReflectionConditions();
 
   std::vector<std::string> names;
+  names.reserve(conditions.size());
   std::transform(conditions.cbegin(), conditions.cend(),
                  std::back_inserter(names), fn);
 
@@ -78,7 +89,7 @@ std::vector<std::string> getAllReflectionConditionSymbols() {
 ReflectionCondition_sptr getReflectionConditionWhere(
     const std::function<bool(const ReflectionCondition_sptr &)> &fn,
     const std::string &hint) {
-  auto conditions = getAllReflectionConditions();
+  const auto & conditions = getAllReflectionConditions();
 
   auto it = std::find_if(conditions.cbegin(), conditions.cend(), fn);
 
diff --git a/Framework/Geometry/test/ReflectionConditionTest.h b/Framework/Geometry/test/ReflectionConditionTest.h
index b4926244594..881123dd648 100644
--- a/Framework/Geometry/test/ReflectionConditionTest.h
+++ b/Framework/Geometry/test/ReflectionConditionTest.h
@@ -8,8 +8,6 @@
 #define MANTID_GEOMETRY_REFLECTIONCONDITIONTEST_H_
 
 #include "MantidGeometry/Crystal/ReflectionCondition.h"
-#include "MantidKernel/System.h"
-#include "MantidKernel/Timer.h"
 #include <cxxtest/TestSuite.h>
 #include <unordered_set>
 
@@ -21,7 +19,7 @@ public:
                size_t count) {
     for (size_t i = 0; i < count; i++) {
       bool v = rc.isAllowed(h[i], k[i], l[i]);
-      TS_ASSERT_EQUALS((valid[i] == 1), v);
+      TS_ASSERT_EQUALS((valid[i] == 1), v)
     }
   }
 
@@ -44,11 +42,11 @@ public:
   }
 
   void test_getAllReflectionConditions() {
-    std::vector<ReflectionCondition_sptr> refs = getAllReflectionConditions();
-    TS_ASSERT_EQUALS(refs.size(), 9);
-    TS_ASSERT(refs[0]);
-    TS_ASSERT_EQUALS(refs[0]->getName(), "Primitive");
-    TS_ASSERT(refs[8]);
+    const auto &refs = getAllReflectionConditions();
+    TS_ASSERT_EQUALS(refs.size(), 9)
+    TS_ASSERT(refs[0])
+    TS_ASSERT_EQUALS(refs[0]->getName(), "Primitive")
+    TS_ASSERT(refs[8])
   }
 
   void test_ReflectionConditionSymbols() {
@@ -63,23 +61,23 @@ public:
     centeringSymbols.insert("Rrev");
     centeringSymbols.insert("H");
 
-    std::vector<ReflectionCondition_sptr> refs = getAllReflectionConditions();
+    const auto &refs = getAllReflectionConditions();
     for (auto &ref : refs) {
       TSM_ASSERT_DIFFERS(ref->getSymbol(),
                          centeringSymbols.find(ref->getSymbol()),
-                         centeringSymbols.end());
+                         centeringSymbols.end())
       centeringSymbols.erase(ref->getSymbol());
     }
 
     // All centering symbols are present if the set is empty.
-    TS_ASSERT_EQUALS(centeringSymbols.size(), 0);
+    TS_ASSERT_EQUALS(centeringSymbols.size(), 0)
   }
 
   void test_getReflectionConditionNames() {
     auto conditions = getAllReflectionConditions();
     auto names = getAllReflectionConditionNames();
 
-    TS_ASSERT_EQUALS(conditions.size(), names.size());
+    TS_ASSERT_EQUALS(conditions.size(), names.size())
 
     // there should not be any duplicates in the names
     std::unordered_set<std::string> nameSet(names.begin(), names.end());
@@ -91,7 +89,7 @@ public:
     auto conditions = getAllReflectionConditions();
     auto symbols = getAllReflectionConditionSymbols();
 
-    TS_ASSERT_EQUALS(conditions.size(), symbols.size());
+    TS_ASSERT_EQUALS(conditions.size(), symbols.size())
 
     // there should not be any duplicates in the names
     std::unordered_set<std::string> symbolSet(symbols.begin(), symbols.end());
@@ -104,11 +102,11 @@ public:
 
     for (auto name : names) {
       TSM_ASSERT_THROWS_NOTHING("Problem with ReflectionCondition: " + name,
-                                getReflectionConditionByName(name));
+                                getReflectionConditionByName(name))
     }
 
     TS_ASSERT_THROWS(getReflectionConditionByName("invalid"),
-                     const std::invalid_argument &);
+                     const std::invalid_argument &)
   }
 
   void test_getReflectionConditionBySymbol() {
@@ -116,11 +114,11 @@ public:
 
     for (auto symbol : symbols) {
       TSM_ASSERT_THROWS_NOTHING("Problem with ReflectionCondition: " + symbol,
-                                getReflectionConditionBySymbol(symbol));
+                                getReflectionConditionBySymbol(symbol))
     }
 
     TS_ASSERT_THROWS(getReflectionConditionBySymbol("Q"),
-                     const std::invalid_argument &);
+                     const std::invalid_argument &)
   }
 };
 
-- 
GitLab