diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument/InfoIteratorBase.h b/Framework/Geometry/inc/MantidGeometry/Instrument/InfoIteratorBase.h index 7eda723ec9cae7d5fe686b272a1f971f788158e5..9e82ae163573aab3475489607d36f0635601af1a 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument/InfoIteratorBase.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument/InfoIteratorBase.h @@ -17,6 +17,10 @@ namespace Geometry { Base to allow users of the Info objects (DetectorInfo etc) access to data via a random access iterator. + +Note that the reference type (InfoItem<T>) causes the iterator to be treated as +std::input_iterator for the purposes of many std algorithms such as +std::advance. See https://en.cppreference.com/w/cpp/iterator/advance for example */ template <typename T, template <typename> class InfoItem> class InfoIteratorBase @@ -51,6 +55,10 @@ private: m_item.m_index + static_cast<size_t>(delta)); } + bool equal(const InfoIteratorBase<T, InfoItem> &other) const { + return getIndex() == other.getIndex(); + } + void increment() { if (m_item.m_index < m_totalSize) { ++m_item.m_index; @@ -67,10 +75,6 @@ private: void setIndex(const size_t index) { m_item.m_index = index; } - bool equal(const InfoIteratorBase<T, InfoItem> &other) const { - return getIndex() == other.getIndex(); - } - InfoItem<T> dereference() const { return m_item; } uint64_t distance_to(const InfoIteratorBase<T, InfoItem> &other) const { diff --git a/Framework/Geometry/test/DetectorInfoIteratorTest.h b/Framework/Geometry/test/DetectorInfoIteratorTest.h index 074ef79ce21e5038e2c9507432dd40fa9a30d71f..6d63c9b6c1ad76b5fc49c4aa3404a813c1b0f408 100644 --- a/Framework/Geometry/test/DetectorInfoIteratorTest.h +++ b/Framework/Geometry/test/DetectorInfoIteratorTest.h @@ -14,9 +14,10 @@ #include "MantidGeometry/Instrument/DetectorInfoIterator.h" #include "MantidGeometry/Instrument/InstrumentVisitor.h" #include "MantidTestHelpers/ComponentCreationHelper.h" -#include <iterator> - #include <cxxtest/TestSuite.h> +#include <iterator> +#include <type_traits> +#include <typeinfo> using namespace ComponentCreationHelper; using namespace Mantid::Geometry; @@ -147,6 +148,35 @@ public: TS_ASSERT(iter == detectorInfo->cbegin()); } + void test_iterator_catagory() { + // Characterisation tests + using ItTag = + typename std::iterator_traits<DetectorInfoConstIt>::iterator_category; + using InputItTag = std::input_iterator_tag; + using BidirectionalItTag = std::bidirectional_iterator_tag; + const static bool inputit = std::is_convertible<ItTag, InputItTag>::value; + const static bool bidirectionalit = + std::is_convertible<ItTag, BidirectionalItTag>::value; + TSM_ASSERT("Iterator expected to be treated as input_iterator", inputit); + // Assert below. Iterator not bidirectional. This is why decrement via + // std::advance is not supported. Iterator reference must be true reference + // to support this. + TSM_ASSERT( + "Iterator expected not to be treated as legacy bidirectional iterator", + !bidirectionalit); + + // see https://en.cppreference.com/w/cpp/iterator/advance + // Demonstrate internal switched behaviour in std::advance + auto detectorInfo = create_detector_info_object(); + auto it = detectorInfo->cbegin(); + TS_ASSERT_EQUALS(it->index(), 0); + std::advance(it, 2); + TS_ASSERT_EQUALS(it->index(), 2); + std::advance(it, -2); + TSM_ASSERT_EQUALS("Was not decremented. Not zero. For reasons above", + it->index(), 2); + } + void test_iterator_advance_and_positions() { // Get the DetectorInfo object auto detectorInfo = create_detector_info_object(); @@ -162,12 +192,12 @@ public: // Go backwards 2 places xValue = 15.0; - std::advance(iter, -2); + iter -= 2; TS_ASSERT_EQUALS(iter->position().X(), xValue) // Go to the start - std::advance(iter, -4); - TS_ASSERT(iter == detectorInfo->cbegin()); + iter -= 4; + TS_ASSERT_EQUALS(iter, detectorInfo->cbegin()); } void test_copy_iterator_and_positions() {