Merge pull request #375 from broadinstitute/intel_iterator_inc_dec

Made return values for variant field iterators consistent with STL iterators

gamgee/individual_field_iterator.h

@@ -169,7 +169,7 @@ class IndividualFieldIterator : public std::iterator<std::random_access_iterator
    * @brief Postfix increment. Advances to the next sample
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result with end(). This is the STL way.
-   * @return the value of the start of the same sample.
+   * @return copy of the current position of the iterator (before the increment)
    */
   IndividualFieldIterator operator++(int) noexcept {
     auto const tmp = IndividualFieldIterator(*this);
@@ -189,12 +189,12 @@ class IndividualFieldIterator : public std::iterator<std::random_access_iterator
   }
 
   /**
-   * @brief Postfix increment.  Reverses to the previous sample.
+   * @brief Postfix decrement.  Reverses to the previous sample.
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the beginning by comparing the result with begin(). This is the STL way.
-   * @return the value of the start of the same sample.
+   * @return copy of the current position of the iterator (before the decrement)
    */
-  IndividualFieldIterator& operator--(int) noexcept {
+  IndividualFieldIterator operator--(int) noexcept {
     auto const tmp = IndividualFieldIterator(*this);
     operator--();
     return tmp;

gamgee/individual_field_value_iterator.h

@@ -209,23 +209,49 @@ class IndividualFieldValueIterator : public std::iterator<std::random_access_ite
    * @brief advances to the next sample
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
+   * @return modified iterator
    */
-  void operator++() noexcept {
+  IndividualFieldValueIterator& operator++() noexcept {
     m_current_data_ptr += m_num_bytes;
     m_is_current_pointee_cached = false;
     m_current_data_ptr = utils::cache_and_advance_to_end_if_necessary(m_current_data_ptr, m_end_data_ptr, *this);
+    return *this;
   }
 
+  /**
+   * @brief postfix operator to advance iterator to the next position
+   * @note mainly designed for iterators
+   * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
+   * @return copy of iterator at current position (before the increment)
+   */
+  IndividualFieldValueIterator operator++(int) noexcept {
+    auto const tmp = IndividualFieldValueIterator(*this);
+    operator++();
+    return tmp;
+  }
   /**
    * @brief advances to the previous sample
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
+   * @return modified iterator
    */
-  void operator--() {
+  IndividualFieldValueIterator& operator--() {
     m_current_data_ptr -= m_num_bytes;
     m_is_current_pointee_cached = false;
+    return *this;
   }
 
+  /**
+   * @brief postfix operator to retreat iterator to the previous position
+   * @note mainly designed for iterators
+   * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
+   * @return copy of iterator at current position (before the decrement)
+   */
+  IndividualFieldValueIterator operator--(int) noexcept {
+    auto const tmp = IndividualFieldValueIterator(*this);
+    operator--();
+    return tmp;
+  }
   /**
    * @brief random access to the value of a given index for reading or writing
    * @param index must be between 0 and the number of indices for this record but no boundary check is done in this implementation

gamgee/shared_field_iterator.h

@@ -119,20 +119,20 @@ class SharedFieldIterator : public std::iterator<std::random_access_iterator_tag
    * @brief advances to the next value
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
-   * @return the next value in it's native type
+   * @return updated iterator
    */
-  VALUE_TYPE operator++() noexcept {
+  SharedFieldIterator& operator++() noexcept {
     m_current_data_ptr += m_bytes_per_value;
     m_is_current_pointee_cached = false;
     m_current_data_ptr = utils::cache_and_advance_to_end_if_necessary(m_current_data_ptr, m_end_data_ptr, *this);
-    return convert_from_byte_array(m_current_data_ptr, 0);
+    return *this;
   }
 
   /**
    * @brief Postfix increment. Advances to the next sample
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result with end(). This is the STL way.
-   * @return the next value in it's native type
+   * @return copy of current iterator before the increment
    */
   SharedFieldIterator operator++(int) noexcept {
     const auto tmp = SharedFieldIterator(*this);
@@ -145,14 +145,25 @@ class SharedFieldIterator : public std::iterator<std::random_access_iterator_tag
    * @brief advances to the previous value
    * @note mainly designed for iterators
    * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result of operator* with end(). This is the STL way.
-   * @return the previous value in it's native type
+   * @return updated iterator
    */
-  VALUE_TYPE operator--() {
+  SharedFieldIterator& operator--() {
     m_current_data_ptr -= m_bytes_per_value;
     m_is_current_pointee_cached = false;
-    return convert_from_byte_array(m_current_data_ptr, 0);
+    return *this;
   }
 
+  /**
+   * @brief Postfix decrement. Move back to the previous sample
+   * @note mainly designed for iterators
+   * @warning does not check for bounds exception, you should verify whether or not you've reached the end by comparing the result with end(). This is the STL way.
+   * @return copy of current iterator before the decrement
+   */
+  SharedFieldIterator operator--(int) noexcept {
+    const auto tmp = SharedFieldIterator(*this);
+    operator--();
+    return tmp;
+  }
   /**
    * @brief difference between two iterators as an integer.
    * @param first is the iterator the position of which is to be subtracted from the position of the current iterator.

test/variant_reader_test.cpp

@@ -437,6 +437,48 @@ void check_operator_index_for_iterators(const Variant& record)
   }
 }
 
+void check_field_iterator_inc_dec(const Variant& record)
+{
+  const auto pl_int  = record.individual_field_as_integer("PL");
+  for(auto pl : pl_int)
+  {
+    for(auto pl_iter = pl.begin();pl_iter != pl.end();)
+    {
+      auto curr_elem = *pl_iter;
+      BOOST_CHECK_EQUAL(*(pl_iter++), curr_elem);
+    }
+    auto pl_next_iter=pl.begin();
+    ++pl_next_iter;
+    for(auto pl_iter = pl.begin();pl_next_iter != pl.end();++pl_next_iter)
+    {
+      auto pl_tmp_iter = pl_next_iter;
+      BOOST_CHECK_EQUAL(*(pl_tmp_iter--), *pl_next_iter);
+      BOOST_CHECK_EQUAL(*pl_tmp_iter, *pl_iter);
+      ++pl_tmp_iter;
+      BOOST_CHECK_EQUAL(*(--pl_tmp_iter), *pl_iter);
+      BOOST_CHECK_EQUAL(*(++pl_iter), *pl_next_iter);
+    }
+  }
+
+  const auto vlint  = record.shared_field_as_integer("VLINT");
+  for(auto vlint_iter = vlint.begin();vlint_iter != vlint.end();)
+  {
+    auto curr_elem = *vlint_iter;
+    BOOST_CHECK_EQUAL(*(vlint_iter++), curr_elem);
+  }
+  auto vlint_next_iter=vlint.begin();
+  ++vlint_next_iter;
+  for(auto vlint_iter = vlint.begin();vlint_next_iter != vlint.end();++vlint_next_iter)
+  {
+    auto vlint_tmp_iter = vlint_next_iter;
+    BOOST_CHECK_EQUAL(*(vlint_tmp_iter--), *vlint_next_iter);
+    BOOST_CHECK_EQUAL(*vlint_tmp_iter, *vlint_iter);
+    ++vlint_tmp_iter;
+    BOOST_CHECK_EQUAL(*(--vlint_tmp_iter), *vlint_iter);
+    BOOST_CHECK_EQUAL(*(++vlint_iter), *vlint_next_iter);
+  }
+}
+
 void check_out_of_bound_exceptions(const Variant& record) {
   const auto vlint_shared = record.integer_shared_field("VLINT");
   const auto af_float_shared  = record.shared_field_as_float("AF");
@@ -607,6 +649,7 @@ void check_individual_field_api(const Variant& record, const uint32_t truth_inde
   check_variable_length_field_api(record, truth_index);
   check_operator_index_for_iterators(record);
   check_out_of_bound_exceptions(record);
+  check_field_iterator_inc_dec(record);
 }
 
 void check_shared_field_api(const Variant& record, const uint32_t truth_index) {