From 07cf86306028aa40b879180a9860fe876c1fdec1 Mon Sep 17 00:00:00 2001
From: Steven Hahn <hahnse@ornl.gov>
Date: Tue, 6 Nov 2018 15:17:30 -0500
Subject: [PATCH] Refactor for simplicity.

---
 Framework/API/inc/MantidAPI/MatrixWorkspace.h |  1 -
 Framework/API/src/MatrixWorkspace.cpp         | 88 ++++++++++++-------
 .../inc/MantidDataObjects/EventWorkspace.h    |  3 -
 .../inc/MantidDataObjects/Workspace2D.h       |  3 -
 Framework/DataObjects/src/EventWorkspace.cpp  | 24 -----
 Framework/DataObjects/src/Workspace2D.cpp     | 24 -----
 .../inc/MantidTestHelpers/FakeObjects.h       | 25 ------
 7 files changed, 54 insertions(+), 114 deletions(-)

diff --git a/Framework/API/inc/MantidAPI/MatrixWorkspace.h b/Framework/API/inc/MantidAPI/MatrixWorkspace.h
index d32afacfefd..088ddf2bf10 100644
--- a/Framework/API/inc/MantidAPI/MatrixWorkspace.h
+++ b/Framework/API/inc/MantidAPI/MatrixWorkspace.h
@@ -585,7 +585,6 @@ private:
   /// A text label for use when plotting spectra
   std::string m_YUnitLabel;
 
-protected:
   /// Flag indicating whether the m_isCommonBinsFlag has been set. False by
   /// default
   mutable bool m_isCommonBinsFlagSet{false};
diff --git a/Framework/API/src/MatrixWorkspace.cpp b/Framework/API/src/MatrixWorkspace.cpp
index fd6bb81feaf..17dbc481480 100644
--- a/Framework/API/src/MatrixWorkspace.cpp
+++ b/Framework/API/src/MatrixWorkspace.cpp
@@ -963,47 +963,67 @@ bool MatrixWorkspace::isHistogramData() const {
  *  @return whether the workspace contains common X bins
  */
 bool MatrixWorkspace::isCommonBins() const {
-  if (!m_isCommonBinsFlagSet) {
-    m_isCommonBinsFlag = true;
-
-    const size_t numHist = getNumberHistograms();
-    // there being only one or zero histograms is accepted as not being an error
-    if (numHist > 1) {
-      const size_t numBins = x(0).size();
-      for (size_t i = 1; i < numHist; ++i) {
-        if (x(i).size() != numBins) {
+  if (m_isCommonBinsFlagSet) {
+    return m_isCommonBinsFlag;
+  }
+
+  m_isCommonBinsFlag = true;
+  const size_t numHist = this->getNumberHistograms();
+  // there being only one or zero histograms is accepted as not being an error
+  if (numHist <= 1) {
+    m_isCommonBinsFlagSet = true;
+    return m_isCommonBinsFlag;
+  }
+
+  // First check if the x-axis shares a common ptr.
+  const HistogramData::HistogramX *first = &x(0);
+  for (size_t i = 1; i < numHist; ++i) {
+    if (&x(i) != first) {
+      m_isCommonBinsFlag = false;
+      break;
+    }
+  }
+
+  // If true, we may return here.
+  if (m_isCommonBinsFlag) {
+    m_isCommonBinsFlagSet = true;
+    return m_isCommonBinsFlag;
+  }
+
+  m_isCommonBinsFlag = true;
+  // Check that that size of each histogram is identical.
+  const size_t numBins = x(0).size();
+  for (size_t i = 1; i < numHist; ++i) {
+    if (x(i).size() != numBins) {
+      m_isCommonBinsFlag = false;
+      break;
+    }
+  }
+
+  // Check that the values of each histogram are identical.
+  if (m_isCommonBinsFlag) {
+    const size_t numBins = x(0).size();
+    const size_t lastSpec = numHist - 1;
+    for (size_t i = 0; i < lastSpec; ++i) {
+      const auto &xi = x(i);
+      const auto &xip1 = x(i + 1);
+      for (size_t j = 0; j < numBins; ++j) {
+        const double first = xi[j];
+        const double next = xip1[j];
+        if (std::abs(first - next) / std::abs(first + next) > 1.0E-9) {
           m_isCommonBinsFlag = false;
           break;
         }
-      }
-
-      // there being only one or zero histograms is accepted as not being an
-      // error
-      if (m_isCommonBinsFlag) {
-        const size_t numBins = x(0).size();
-        const size_t lastSpec = numHist - 1;
-        for (size_t i = 0; i < lastSpec; ++i) {
-          const auto &xi = x(i);
-          const auto &xip1 = x(i + 1);
-          for (size_t j = 0; j < numBins; ++j) {
-            const double first = xi[j];
-            const double next = xip1[j];
-            if (std::abs(first - next) / std::abs(first + next) > 1.0E-9) {
-              m_isCommonBinsFlag = false;
-              break;
-            }
-            // handle Nan's and inf's
-            if ((std::isinf(first) != std::isinf(next)) ||
-                (std::isnan(first) != std::isnan(next))) {
-              m_isCommonBinsFlag = false;
-              break;
-            }
-          }
+        // handle Nan's and inf's
+        if ((std::isinf(first) != std::isinf(next)) ||
+            (std::isnan(first) != std::isnan(next))) {
+          m_isCommonBinsFlag = false;
+          break;
         }
       }
     }
-    m_isCommonBinsFlagSet = true;
   }
+  m_isCommonBinsFlagSet = true;
   return m_isCommonBinsFlag;
 }
 
diff --git a/Framework/DataObjects/inc/MantidDataObjects/EventWorkspace.h b/Framework/DataObjects/inc/MantidDataObjects/EventWorkspace.h
index 4c3daf5d0e4..f531ab34e85 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/EventWorkspace.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/EventWorkspace.h
@@ -151,9 +151,6 @@ public:
                             const bool entireRange) const override;
   EventWorkspace &operator=(const EventWorkspace &other) = delete;
 
-  /// Returns true if the workspace contains has common X bins
-  bool isCommonBins() const override;
-
 protected:
   /// Protected copy constructor. May be used by childs for cloning.
   EventWorkspace(const EventWorkspace &other);
diff --git a/Framework/DataObjects/inc/MantidDataObjects/Workspace2D.h b/Framework/DataObjects/inc/MantidDataObjects/Workspace2D.h
index 34082b267c8..acc8dddc9f7 100644
--- a/Framework/DataObjects/inc/MantidDataObjects/Workspace2D.h
+++ b/Framework/DataObjects/inc/MantidDataObjects/Workspace2D.h
@@ -80,9 +80,6 @@ public:
                      bool loadAsRectImg = false, double scale_1 = 1.0,
                      bool parallelExecution = true);
 
-  /// Returns true if the workspace contains has common X bins
-  bool isCommonBins() const override;
-
 protected:
   /// Protected copy constructor. May be used by childs for cloning.
   Workspace2D(const Workspace2D &other);
diff --git a/Framework/DataObjects/src/EventWorkspace.cpp b/Framework/DataObjects/src/EventWorkspace.cpp
index b151573b39c..d9ac64c79e8 100644
--- a/Framework/DataObjects/src/EventWorkspace.cpp
+++ b/Framework/DataObjects/src/EventWorkspace.cpp
@@ -605,30 +605,6 @@ void EventWorkspace::resetAllXToSingleBin() {
   setAllX({tofmin, tofmax});
 }
 
-bool EventWorkspace::isCommonBins() const {
-  if (!m_isCommonBinsFlagSet) {
-    m_isCommonBinsFlag = true;
-    const size_t numHist = this->getNumberHistograms();
-    // there being only one or zero histograms is accepted as not being an error
-    if (numHist > 1) {
-      // First check if the x-axis shares a common cow_ptr.
-      auto first = data[0]->ptrX();
-      for (const auto &el : data) {
-        if (el->ptrX() != first) {
-          m_isCommonBinsFlag = false;
-          break;
-        }
-      }
-      // If false, we need to check more carefully.
-      if (!m_isCommonBinsFlag) {
-        return MatrixWorkspace::isCommonBins();
-      }
-    }
-    m_isCommonBinsFlagSet = true;
-  }
-  return m_isCommonBinsFlag;
-}
-
 /** Task for sorting an event list */
 class EventSortingTask {
 public:
diff --git a/Framework/DataObjects/src/Workspace2D.cpp b/Framework/DataObjects/src/Workspace2D.cpp
index ef01141f435..2a016c9c954 100644
--- a/Framework/DataObjects/src/Workspace2D.cpp
+++ b/Framework/DataObjects/src/Workspace2D.cpp
@@ -362,30 +362,6 @@ Workspace2D *Workspace2D::doClone() const { return new Workspace2D(*this); }
 Workspace2D *Workspace2D::doCloneEmpty() const {
   return new Workspace2D(storageMode());
 }
-
-bool Workspace2D::isCommonBins() const {
-  if (!m_isCommonBinsFlagSet) {
-    m_isCommonBinsFlag = true;
-    const size_t numHist = this->getNumberHistograms();
-    // there being only one or zero histograms is accepted as not being an error
-    if (numHist > 1) {
-      // First check if the x-axis shares a common cow_ptr.
-      auto first = data[0]->ptrX();
-      for (const auto &histogram : data) {
-        if (histogram->ptrX() != first) {
-          m_isCommonBinsFlag = false;
-          break;
-        }
-      }
-      // If false, we need to check more carefully.
-      if (!m_isCommonBinsFlag) {
-        return MatrixWorkspace::isCommonBins();
-      }
-    }
-    m_isCommonBinsFlagSet = true;
-  }
-  return m_isCommonBinsFlag;
-}
 } // namespace DataObjects
 } // namespace Mantid
 
diff --git a/Framework/TestHelpers/inc/MantidTestHelpers/FakeObjects.h b/Framework/TestHelpers/inc/MantidTestHelpers/FakeObjects.h
index 69c47b0dba7..e961c55ef80 100644
--- a/Framework/TestHelpers/inc/MantidTestHelpers/FakeObjects.h
+++ b/Framework/TestHelpers/inc/MantidTestHelpers/FakeObjects.h
@@ -199,31 +199,6 @@ protected:
     throw std::runtime_error(
         "Cloning of AxeslessWorkspaceTester is not implemented.");
   }
-public:
-  bool isCommonBins() const override {
-    if (!m_isCommonBinsFlagSet) {
-      m_isCommonBinsFlag = true;
-      const size_t numHist = this->getNumberHistograms();
-      // there being only one or zero histograms is accepted as not being an
-      // error
-      if (numHist > 1) {
-        // First check if the x-axis shares a common cow_ptr.
-        auto first = m_vec[0].ptrX();
-        for (const auto &st : m_vec) {
-          if (st.ptrX() != first) {
-            m_isCommonBinsFlag = false;
-            break;
-          }
-        }
-        // If false, we need to check more carefully.
-        if (!m_isCommonBinsFlag) {
-          return MatrixWorkspace::isCommonBins();
-        }
-      }
-      m_isCommonBinsFlagSet = true;
-    }
-    return m_isCommonBinsFlag;
-  }
 
 private:
   std::vector<SpectrumTester> m_vec;
-- 
GitLab