refactor: improve use of facade in zip/indexed iterators

include/iterator/indexed_iterator.hpp

@@ -9,100 +9,40 @@
 
 #include <iterator>
 
-#include "detail/arrow_proxy.h"
+#include "iterator/facade.h"
 
 namespace iterator {
-  template <typename Iterator> class indexed_iterator {
-  private:
-    using base_value_type = typename std::iterator_traits<Iterator>::value_type;
-    using base_reference = typename std::iterator_traits<Iterator>::reference;
-
+  template <typename It>
+  class indexed_iterator : public facade<indexed_iterator<It>> {
   public:
-    using index_type = std::size_t;
-    using value_type = std::pair<index_type, base_value_type>;
-    using reference = std::pair<index_type, base_reference>;
-    using pointer = detail::arrow_proxy<reference>;
-    using difference_type =
-        typename std::iterator_traits<Iterator>::difference_type;
-    using iterator_category =
-        typename std::iterator_traits<Iterator>::iterator_category;
-
-    indexed_iterator() : _base(), _index(0) {}
-    indexed_iterator(Iterator base, index_type idx = 0)
-        : _base(base), _index(idx) {}
+    using reference = std::pair<size_t, typename std::iterator_traits<It>::reference>;
+    using difference_type = typename std::iterator_traits<It>::difference_type;
+  public:
+    indexed_iterator() = default;
+    explicit indexed_iterator(It base, size_t idx = 0) : _base(base), _index(idx) {}
 
-    template <typename OtherIterator>
-    indexed_iterator(indexed_iterator<OtherIterator> const & oiter)
+    template <typename O> indexed_iterator(indexed_iterator<O> const & oiter)
         : _base(oiter._base), _index(oiter._index) {}
 
-    reference operator*() const { return reference{_index, *_base}; }
-    pointer operator->() const { return {operator*()}; }
-
-    indexed_iterator & operator++() {
-      ++_base;
-      ++_index;
-      return *this;
-    }
-    indexed_iterator operator++(int) {
-      indexed_iterator tmp{*this};
-      operator++();
-      return tmp;
-    }
-    bool operator==(indexed_iterator const & other) const {
-      return _base == other._base;
-    }
-    bool operator!=(indexed_iterator const & other) const {
-      return _base != other._base;
-    }
-
-    // Requires: iterator_category = bidirectional_iterator_tag
-    indexed_iterator & operator--() {
-      --_base;
-      --_index;
-      return *this;
-    }
-    indexed_iterator operator--(int) {
-      indexed_iterator tmp{*this};
-      operator--();
-      return tmp;
+    reference dereference() const { return {_index, *_base}; }
+    
+    void advance(difference_type off) {
+      _base += off;
+      _index += off;
     }
 
-    // Requires: iterator_category = random_access_iterator_tag
-    indexed_iterator operator+(difference_type n) const {
-      return indexed_iterator{*this} += n;
-    }
-    indexed_iterator & operator+=(difference_type n) {
-      _index += n;
-      _base += n;
-      return *this;
-    }
-    difference_type operator-(indexed_iterator const & it) const {
-      return _base - it._base;
-    }
-    indexed_iterator operator-(difference_type n) const {
-      return indexed_iterator{*this} -= n;
-    }
-    indexed_iterator & operator-=(difference_type n) {
-      _index -= n;
-      _base -= n;
-      return *this;
-    }
-    bool operator<=(indexed_iterator const & other) const {
-      return _base <= other._base;
-    }
-    bool operator<(indexed_iterator const & other) const {
-      return _base < other._base;
-    }
-    bool operator>=(indexed_iterator const & other) const {
-      return _base >= other._base;
-    }
-    bool operator>(indexed_iterator const & other) const {
-      return _base > other._base;
+    // SFINAE means that if Iterator is not random access, then this still works
+    // TODO: Investigate using _index for comparisons instead of _base
+    bool equal_to(indexed_iterator const & other) const { return _base == other._base; }
+    difference_type distance_to(indexed_iterator const & other) const {
+      return other._base - _base;
     }
 
   private:
-    template <typename It> friend class ::iterator::indexed_iterator;
-    Iterator _base;
-    index_type _index;
+    template <typename O> friend class indexed_iterator;
+    It _base;
+    size_t _index{0};
   };
 }
+
+MAKE_ITERATOR_FACADE_TYPEDEFS_T(::iterator::indexed_iterator)

include/iterator/zip_iterator.hpp

@@ -3,80 +3,71 @@
 #include <iterator>
 #include <tuple>
 
-#include "proxy.h"
-
-namespace iterator::zip {
-  template <typename... Iters> struct impl {
-  public:
-    using difference_type = std::common_type_t<
-        typename std::iterator_traits<Iters>::difference_type...>;
-
-  public:
-    std::tuple<Iters...> data;
-
-  public:
-    impl() = default;
-    impl(Iters &&... iters) : data(iters...) {}
-
-    auto operator*() const {
-      return std::make_tuple(*std::get<Iters>(data)...);
-    }
-
-    void operator++(int) {
-      [[maybe_unused]] auto l = {(++(std::get<Iters>(data)), 0)...};
-    }
-
-    void operator--(int) {
-      [[maybe_unused]] auto l = {(--(std::get<Iters>(data)), 0)...};
-    }
-
-    void operator+=(difference_type d) {
-      [[maybe_unused]] auto l = {((std::get<Iters>(data) += d), 0)...};
-    }
-
-    bool operator==(impl const & other) const { return data == other.data; }
-
-    auto operator-(impl const & other) const {
-      return std::get<0>(data) - std::get<0>(other.data);
-    }
-  };
-}
-
-namespace std {
-  template <typename... Iters>
-  struct iterator_traits<::iterator::zip::impl<Iters...>> {
-    using difference_type =
-        common_type_t<typename iterator_traits<Iters>::difference_type...>;
-    using iterator_category =
-        common_type_t<typename iterator_traits<Iters>::iterator_category...>;
-  };
+#include "facade.h"
+
+namespace iterator::detail {
+template <typename Tuple, typename IS> class zip_iterator_impl;
+
+template <typename... Ts, size_t... Is>
+class zip_iterator_impl<std::tuple<Ts...>, std::index_sequence<Is...>> {
+public:
+  using difference_type =
+      std::common_type_t<typename std::iterator_traits<Ts>::difference_type...>;
+
+public:
+  zip_iterator_impl() = default;
+  zip_iterator_impl(Ts... iters) : _data(iters...) {}
+  
+  auto dereference() const {
+    return std::forward_as_tuple(*std::get<Is>(_data)...);
+  }
+
+  void increment() {
+    [[maybe_unused]] auto l = {((++std::get<Is>(_data)), 0)...};
+  }
+  void decrement() {
+    [[maybe_unused]] auto l = {((++std::get<Is>(_data)), 0)...};
+  }
+  void advance(difference_type d) {
+    [[maybe_unused]] auto l = {((std::get<Is>(_data) += d), 0)...};
+  }
+
+  bool equal_to(zip_iterator_impl const & other) const { return _data == other._data; }
+  
+  auto distance_to(zip_iterator_impl const & other) const {
+    return std::get<0>(other._data) - std::get<0>(_data);
+  }
+  
+private:
+  std::tuple<Ts...> _data;
+};
 }
 
 namespace iterator {
-  template <typename... Iters>
-  class zip_iterator
-      : public proxy<zip::impl<Iters...>, zip_iterator<Iters...>> {
-  private:
-    using super = proxy<zip::impl<Iters...>, zip_iterator<Iters...>>;
+  template <typename... Ts>
+  using index_sequence = decltype(std::make_index_sequence<sizeof...(Ts)>{});
+  template <typename... Ts>
+  using zip_impl = detail::zip_iterator_impl<std::tuple<Ts...>, index_sequence<Ts...>>;
 
+  template <typename... Iters>
+  class zip_iterator : public zip_impl<Iters...>, public facade<zip_iterator<Iters...>> {
   public:
     zip_iterator() = default;
-    zip_iterator(Iters &&... iters) : super(std::forward<Iters>(iters)...) {}
+    zip_iterator(Iters... iters) : zip_impl<Iters...>(iters...) {}
   };
-}
 
-namespace std {
-  template <typename... T>
-  struct iterator_traits<::iterator::zip_iterator<T...>>
-      : std::iterator_traits<
-            ::iterator::facade<::iterator::zip_iterator<T...>>> {
-    // This shouldn't need to be implemented, but for some reason my traits
-    // are not correctly deducing here.
-    using iterator_category =
-        common_type_t<typename iterator_traits<T>::iterator_category...>;
-  };
+  template <typename... It> zip_iterator(It...) -> zip_iterator<It...>;
 }
 
+template <typename... T>
+struct std::iterator_traits<::iterator::zip_iterator<T...>>
+    : std::iterator_traits<::iterator::facade<::iterator::zip_iterator<T...>>> {
+  // This shouldn't need to be implemented, but for some reason my traits
+  // are not correctly deducing here.
+  using iterator_category =
+      common_type_t<typename iterator_traits<T>::iterator_category...>;
+};
+
 template <typename... Is>
 iterator::zip_iterator<Is...> make_zip_iterator(Is &&... iters) {
   return {std::forward<Is>(iters)...};