From 285d4da034f8f1589018160607b2acb001c046ac Mon Sep 17 00:00:00 2001
From: Samuel Jackson <samueljackson@outlook.com>
Date: Mon, 6 Nov 2017 16:05:07 +0000
Subject: [PATCH] Refs #21130 Refactoring & adding some documentation

---
 .../inc/MantidHistogramData/HistogramItem.h           | 11 +++++++++++
 .../inc/MantidHistogramData/HistogramIterator.h       |  6 +++++-
 Framework/HistogramData/src/HistogramIterator.cpp     |  3 ++-
 Framework/HistogramData/test/HistogramItemTest.h      | 10 +++++-----
 4 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Framework/HistogramData/inc/MantidHistogramData/HistogramItem.h b/Framework/HistogramData/inc/MantidHistogramData/HistogramItem.h
index e508ce87bdc..bb3ec68a0c7 100644
--- a/Framework/HistogramData/inc/MantidHistogramData/HistogramItem.h
+++ b/Framework/HistogramData/inc/MantidHistogramData/HistogramItem.h
@@ -14,6 +14,17 @@ class Histogram;
 
 /** HistogramItem
 
+  HistogramItem represents a single index in a Histogram object.
+
+  HistogramItem is the type that is returned when iterating over a
+  Histogram using the foreach loop syntax. HistogramItem provides
+  efficient access to a single point in the Histogram.
+
+  HistogramItem will only perform conversions between counts and 
+  frequencies or points and bins when explicitly told to. Code that
+  requires only a few values from a large Histogram may find this faster
+  than converting the whole X, Y or E value. 
+
   @author Samuel Jackson
   @date 2017
 
diff --git a/Framework/HistogramData/inc/MantidHistogramData/HistogramIterator.h b/Framework/HistogramData/inc/MantidHistogramData/HistogramIterator.h
index 922d4d1e895..dd574c0b350 100644
--- a/Framework/HistogramData/inc/MantidHistogramData/HistogramIterator.h
+++ b/Framework/HistogramData/inc/MantidHistogramData/HistogramIterator.h
@@ -13,6 +13,10 @@ namespace Mantid {
 namespace HistogramData {
 
 /** HistogramIterator
+ 
+  HistogramIterator implements an the iterator interface for HistogramData.
+  At each position the iterator will point to an instance of a HistogramItem.
+  This item provides direct access to the values at a particular index.
 
   @author Samuel Jackson
   @date 2017
@@ -53,7 +57,7 @@ private:
     bool equal(const HistogramIterator &other) const;
     HistogramItem& dereference() const;
     void decrement();
-    void advance(uint64_t delta);
+    void advance(int64_t delta);
     uint64_t distance_to(const HistogramIterator &other) const;
 
     const Histogram& m_histogram;
diff --git a/Framework/HistogramData/src/HistogramIterator.cpp b/Framework/HistogramData/src/HistogramIterator.cpp
index 42dca15cc26..d6c6f2cc414 100644
--- a/Framework/HistogramData/src/HistogramIterator.cpp
+++ b/Framework/HistogramData/src/HistogramIterator.cpp
@@ -30,13 +30,14 @@ void HistogramIterator::decrement() {
     }
 }
 
-void HistogramIterator::advance(uint64_t delta) {
+void HistogramIterator::advance(int64_t delta) {
     m_index = delta < 0 ? std::max(static_cast<uint64_t>(0),
             static_cast<uint64_t>(m_index) + delta)
         : std::min(m_histogram.size(),
                 m_index + static_cast<size_t>(delta));
     m_currentItem = make_unique<HistogramItem>(m_histogram, m_index); 
 }
+
 uint64_t HistogramIterator::distance_to(const HistogramIterator &other) const {
     return static_cast<uint64_t>(other.m_index) -
         static_cast<uint64_t>(m_index);
diff --git a/Framework/HistogramData/test/HistogramItemTest.h b/Framework/HistogramData/test/HistogramItemTest.h
index f05f1dec2b0..5e50695ded1 100644
--- a/Framework/HistogramData/test/HistogramItemTest.h
+++ b/Framework/HistogramData/test/HistogramItemTest.h
@@ -126,29 +126,29 @@ public:
   void test_get_binEdges_from_histogram_with_bins() {
     Histogram hist(BinEdges{0.1, 0.2, 0.4, 0.5}, Frequencies{1, 2, 4});
     HistogramItem item(hist, 1);
-    compare(item.binEdges(), BinEdges{0.2, 0.4}, tolerance);
+    compare(item.binEdges(), BinEdges{0.2, 0.4});
   }
 
   void test_get_binEdges_from_histogram_with_points() {
     Histogram hist(Points{0.1, 0.2, 0.4}, Frequencies{1, 2, 4});
     HistogramItem item(hist, 1);
-    compare(item.binEdges(), BinEdges{0.15, 0.3}, tolerance);
+    compare(item.binEdges(), BinEdges{0.15, 0.3});
   }
 
   void test_get_point_from_histogram_with_bins() {
     Histogram hist(BinEdges{0.1, 0.2, 0.4, 0.5}, Frequencies{1, 2, 4});
     HistogramItem item(hist, 1);
-    compare(item.point(), Points{0.3}, tolerance);
+    compare(item.point(), Points{0.3});
   }
 
   void test_get_point_from_histogram_with_points() {
     Histogram hist(Points{0.1, 0.2, 0.4}, Frequencies{1, 2, 4});
     HistogramItem item(hist, 1);
-    compare(item.point(), Points{0.2}, tolerance);
+    compare(item.point(), Points{0.2});
   }
 
   template<typename T>
-  void compare(const T& lhs, const T& rhs, double tol = 0) {
+  void compare(const T& lhs, const T& rhs) {
       TS_ASSERT_EQUALS(lhs.size(), rhs.size())
 
       for (size_t i = 0; i <  lhs.size(); ++i) {
-- 
GitLab