Browse Source

Make more things facades.

Sam Jaffe 4 years ago
parent
commit
d7631b5002

+ 8 - 0
include/iterator/detail/traits.h

@@ -0,0 +1,8 @@
+#pragma once
+
+#define _val(type) std::declval<type>()
+#define exists(expr) void_t<decltype(expr)>
+
+namespace iterator::detail {
+  template <typename> using void_t = void;
+}

+ 23 - 47
include/iterator/end_aware_iterator.hpp

@@ -9,76 +9,52 @@
 
 #include "iterator_fwd.hpp"
 
-#include <iterator>
+#include "facade.h"
 
 namespace iterator {
   /**
    * @class end_aware_iterator
    * @brief An iterator that keeps track of the relative end of the range.
    *
-   * @tparam Iterator The underlying iterator type
+   * @tparam It The underlying iterator type
    */
-  template <typename Iterator> class end_aware_iterator {
+  template <typename It>
+  class end_aware_iterator : public facade<end_aware_iterator<It>> {
   public:
-    using iter_type = Iterator;
-    using value_type = typename std::iterator_traits<iter_type>::value_type;
-    using reference = typename std::iterator_traits<iter_type>::reference;
-    using pointer = typename std::iterator_traits<iter_type>::pointer;
-    using difference_type =
-        typename std::iterator_traits<iter_type>::difference_type;
-    using iterator_category = std::forward_iterator_tag;
+    using value_type = typename It::value_type;
+    struct sentinel_type {};
+    static constexpr sentinel_type sentinel;
 
   public:
     end_aware_iterator() = default;
-    end_aware_iterator(iter_type it, iter_type end) : curr_(it), end_(end) {}
-    end_aware_iterator(iter_type end) : curr_(end), end_(end) {}
+    end_aware_iterator(It it, It end) : curr_(it), end_(end) {}
+    end_aware_iterator(It end) : curr_(end), end_(end) {}
 
     template <typename I>
     end_aware_iterator(end_aware_iterator<I> const & other)
         : curr_(other.current()), end_(other.end()) {}
 
-    end_aware_iterator & operator++() {
-      if (!done()) { ++curr_; }
-      return *this;
+    decltype(auto) dereference() const { return *curr_; }
+    void increment() { ++curr_; }
+    bool at_end() const { return curr_ == end_; }
+    bool equal_to(end_aware_iterator const & other) const {
+      // TODO: Fix this clause
+      return (at_end() && other.at_end()) || curr_ == other.curr_;
     }
 
-    end_aware_iterator operator++(int) {
-      end_aware_iterator tmp{*this};
-      operator++();
-      return tmp;
-    }
-
-    reference operator*() const { return *curr_; }
-    pointer operator->() const { return std::addressof(*curr_); }
-
-    bool done() const { return curr_ == end_; }
-    /**
-     * When comparing iterators that do not point to the same collection/range,
-     * the result is not specified by the standard. Therefore, as a matter of
-     * convenience, even if the underlying data is different, all end-aware
-     * iterators in the done() state are considered equal.
-     *
-     * @see done
-     * @return true if both iterators are done, or if the underlying iterators
-     * are logically equal
-     */
-    bool operator==(end_aware_iterator const & other) const {
-      return (done() && other.done()) ||
-             (curr_ == other.curr_ && end_ == other.end_);
-    }
-
-    bool operator!=(end_aware_iterator const & other) {
-      return !(operator==(other));
-    }
-
-    iter_type current() const { return curr_; }
-    iter_type end() const { return end_; }
+    It current() const { return curr_; }
+    It end() const { return end_; }
 
   private:
-    iter_type curr_, end_;
+    It curr_, end_;
   };
+
+  template <typename It> end_aware_iterator(It) -> end_aware_iterator<It>;
+  template <typename It> end_aware_iterator(It, It) -> end_aware_iterator<It>;
 }
 
+MAKE_ITERATOR_FACADE_TYPEDEFS_T(::iterator::end_aware_iterator);
+
 template <typename Iter>
 iterator::end_aware_iterator<Iter> make_end_aware_iterator(Iter a, Iter b) {
   return iterator::end_aware_iterator<Iter>(a, b);

+ 25 - 5
include/iterator/facade.h

@@ -4,13 +4,9 @@
 #include <type_traits>
 
 #include "detail/arrow_proxy.h"
-
-#define _val(type) std::declval<type>()
-#define exists(expr) void_t<decltype(expr)>
+#include "detail/traits.h"
 
 namespace iterator::detail {
-  template <typename> using void_t = void;
-
   template <typename, typename = void> struct has_equal_to : std::false_type {};
   template <typename T>
   struct has_equal_to<T, exists(_val(T).equal_to(_val(T)))> : std::true_type {};
@@ -52,7 +48,17 @@ namespace iterator::detail {
     using type = decltype(_val(T).distance_to(_val(T)));
   };
 
+  template <typename, typename = void> struct sentinal_type {
+    using type = void;
+  };
+
+  template <typename T>
+  struct sentinal_type<T, void_t<typename T::sentinal_type>> {
+    using type = typename T::sentinal_type;
+  };
+
   template <typename T> using distance_to_t = typename distance_to<T>::type;
+  template <typename T> using sentinal_type_t = typename sentinal_type<T>::type;
   template <typename T> constexpr bool has_equal_to_v = has_equal_to<T>{};
   template <typename T> constexpr bool has_distance_to_v = has_distance_to<T>{};
   template <typename T> constexpr bool has_increment_v = has_increment<T>{};
@@ -88,6 +94,10 @@ namespace iterator {
   using difference_type_arg_t =
       std::enable_if_t<std::is_convertible_v<D, detail::distance_to_t<T>>>;
 
+  template <typename D, typename T>
+  using sentinel_type_arg_t =
+      std::enable_if_t<std::is_same_v<D, detail::sentinal_type_t<T>>>;
+
   template <typename self_type> class facade {
   public:
     decltype(auto) operator*() const { return self().dereference(); }
@@ -188,6 +198,16 @@ namespace iterator {
       }
     }
 
+    template <typename S, typename = sentinel_type_arg_t<S, self_type>>
+    friend auto operator==(self_type self, S) {
+      return self.at_end();
+    }
+
+    template <typename S, typename = sentinel_type_arg_t<S, self_type>>
+    friend auto operator!=(self_type self, S) {
+      return !self.at_end();
+    }
+
     friend bool operator!=(self_type const & left, self_type const & right) {
       return !(left == right);
     }

+ 23 - 28
include/iterator/filter_iterator.hpp

@@ -10,53 +10,48 @@
 #include <functional>
 
 #include "end_aware_iterator.hpp"
+#include "facade.h"
 
 namespace iterator {
   template <typename Iter>
-  class filter_iterator : public end_aware_iterator<Iter> {
-  private:
-    using super = end_aware_iterator<Iter>;
-
+  class filter_iterator : public facade<filter_iterator<Iter>> {
   public:
-    using value_type = typename super::value_type;
-    using reference = typename super::reference;
-    using pointer = typename super::pointer;
-    using difference_type = typename super::difference_type;
-    using iterator_category = typename super::iterator_category;
+    using super = end_aware_iterator<Iter>;
+    using reference = decltype(*std::declval<super>());
+    using value_type = std::remove_cv_t<std::remove_reference_t<reference>>;
+    using difference_type = std::ptrdiff_t;
+    using iterator_category = std::forward_iterator_tag;
 
   public:
     filter_iterator() = default;
     template <typename... Args>
     filter_iterator(std::function<bool(value_type const &)> && p,
                     Args &&... super_args)
-        : super(std::forward<Args>(super_args)...), pred(std::move(p)) {
-      if (should_advance()) { advance(); }
-    }
-
-    filter_iterator & operator++() {
-      advance();
-      return *this;
-    }
-
-    filter_iterator operator++(int) {
-      filter_iterator tmp{*this};
-      operator++();
-      return tmp;
+        : base(end_aware_iterator(std::forward<Args>(super_args)...)),
+          pred(std::move(p)) {
+      if (should_advance()) { increment(); }
     }
 
-  private:
-    bool should_advance() {
-      return !super::done() && !pred(super::operator*());
-    }
-    void advance() {
+    decltype(auto) dereference() const { return base.dereference(); }
+    void increment() {
       do {
-        super::operator++();
+        base.increment();
       } while (should_advance());
     }
+    bool equal_to(filter_iterator const & other) const {
+      return base == other.base;
+    }
+
+  public:
+    bool should_advance() { return !base.at_end() && !pred(dereference()); }
+
+    end_aware_iterator<Iter> base;
     std::function<bool(value_type const &)> pred;
   };
 }
 
+MAKE_ITERATOR_FACADE_TYPEDEFS_T(::iterator::filter_iterator);
+
 template <typename Pred, typename Iter>
 iterator::filter_iterator<Iter> make_filter_iterator(Pred && p, Iter it,
                                                      Iter end) {

+ 10 - 21
include/iterator/join_iterator.hpp

@@ -12,8 +12,11 @@
 #include <iterator>
 #include <utility>
 
+#include "facade.h"
+
 namespace iterator {
-  template <typename MetaIterator> class joining_iterator {
+  template <typename MetaIterator>
+  class joining_iterator : public facade<joining_iterator<MetaIterator>> {
   public:
     using join_iter = MetaIterator;
     using joinable_type = typename join_iter::value_type;
@@ -41,42 +44,28 @@ namespace iterator {
       if (update) { update_iterator(); }
     }
 
-    joining_iterator & operator++() {
-      if ((++iterator_).done()) {
+    void increment() {
+      if ((iterator_++).at_end()) {
         ++joiner_;
         update_iterator();
       }
-      return *this;
-    }
-
-    joining_iterator operator++(int) {
-      joining_iterator tmp{*this};
-      operator++();
-      return tmp;
     }
 
-    reference operator*() const { return iterator_.operator*(); }
-    pointer operator->() const { return iterator_.operator->(); }
+    decltype(auto) dereference() const { return *iterator_; }
 
-    bool operator==(joining_iterator const & other) const {
+    bool equal_to(joining_iterator const & other) const {
       return joiner_ == other.joiner_ && iterator_ == other.iterator_;
     }
 
-    bool operator!=(joining_iterator const & other) const {
-      return !(operator==(other));
-    }
-
     end_aware_iterator<join_iter> join_iterator() const { return joiner_; }
     end_aware_iterator<iter_type> element_iterator() const { return iterator_; }
 
   private:
     void update_iterator() {
-      while (!joiner_.done() && std::begin(*joiner_) == std::end(*joiner_)) {
+      while (!joiner_.at_end() && make_end_aware_iterator(*joiner_).at_end()) {
         ++joiner_;
       }
-      if (!joiner_.done()) {
-        iterator_ = {std::begin(*joiner_), std::end(*joiner_)};
-      }
+      if (!joiner_.at_end()) { iterator_ = make_end_aware_iterator(*joiner_); }
     }
 
     end_aware_iterator<join_iter> joiner_;

+ 6 - 7
test/end_aware_iterator_test.cxx

@@ -36,19 +36,19 @@ TEST(EndAwareIteratorTest, MutableActionsArePassthrough) {
 TEST(EndAwareIteratorTest, CanTellYouThatItsReachedEnd) {
   std::vector<int> v{1, 2, 3, 4, 5};
   end_aware_iterator it{v.end() - 1, v.end()};
-  EXPECT_FALSE(it.done());
+  EXPECT_FALSE(it.at_end());
   ++it;
   EXPECT_THAT(it, end_aware_iterator(v.end(), v.end()));
-  EXPECT_TRUE(it.done());
+  EXPECT_TRUE(it.at_end());
 }
 
 TEST(EndAwareIteratorTest, SingleArgIsEnd) {
   std::vector<int> v{1, 2, 3, 4, 5};
-  EXPECT_TRUE(end_aware_iterator(v.begin()).done());
+  EXPECT_TRUE(end_aware_iterator(v.begin()).at_end());
 }
 
 TEST(EndAwareIteratorTest, EmptyIteratorIsEnd) {
-  EXPECT_TRUE(end_aware_iterator().done());
+  EXPECT_TRUE(end_aware_iterator().at_end());
 }
 
 TEST(EndAwareIteratorTest, AllEndPointsAreEqual) {
@@ -74,11 +74,10 @@ TEST(EndAwareIteratorTest, PostIncrementReturnsCopyOfPrev) {
   EXPECT_THAT(*eai, 2);
 }
 
-TEST(EndAwareIteratorTest, IncrementOnEndIsNoOp) {
+TEST(EndAwareIteratorTest, IncrementOnEndIsUnsafe) {
   std::vector<int> v{1, 2, 3, 4, 5};
   end_aware_iterator it{v.end(), v.end()};
   end_aware_iterator const cp = it;
   ++it;
-  EXPECT_THAT(it, cp);
-  EXPECT_TRUE(it.done());
+  EXPECT_NE(it, cp);
 }

+ 2 - 2
test/filter_iterator_test.cxx

@@ -42,7 +42,7 @@ TEST(FilterIteratorTest, IfNonMatchThenStartIsEnd) {
   EXPECT_THAT(it, end);
 }
 
-TEST(FilterIteratorTest, IncrementEndIsNoOp) {
+TEST(FilterIteratorTest, IncrementOnEndIsUnsafe) {
   std::vector<int> const data = {1, 2, 3, 4, 5};
   auto pred = [](int i) { return i % 2 == 0; };
   auto it = make_filter_iterator(pred, data);
@@ -50,5 +50,5 @@ TEST(FilterIteratorTest, IncrementEndIsNoOp) {
   ++++it;
   EXPECT_THAT(it, end);
   ++it;
-  EXPECT_THAT(it, end);
+  EXPECT_NE(it, end);
 }

+ 6 - 4
test/join_iterator_test.cxx

@@ -70,20 +70,22 @@ TEST(JoinIteratorTest, PostIncrementReturnsCopyOfPrev) {
 
 TEST(JoinIteratorTest, MovesFromListToListWhenReachingEnd) {
   std::vector<std::vector<int>> mv{{1, 2, 3}, {4, 5, 6}};
-  join_iterator it(make_end_aware_iterator(mv), {mv[0].end(), mv[0].end()});
+  join_iterator it(make_end_aware_iterator(mv));
+  std::advance(it, 3);
   EXPECT_THAT(*++it, mv[1][0]);
 }
 
 TEST(JoinIteratorTest, SkipsOverEmptyElements) {
   std::vector<std::vector<int>> mv{{1, 2, 3}, {}, {4, 5, 6}};
-  join_iterator it(make_end_aware_iterator(mv), {mv[0].end(), mv[0].end()});
+  join_iterator it(make_end_aware_iterator(mv));
+  std::advance(it, 3);
   EXPECT_THAT(*++it, mv[2][0]);
 }
 
-TEST(JoinIteratorTest, IncrementEndIsNoOp) {
+TEST(JoinIteratorTest, IncrementEndIsUnsafe) {
   std::vector<std::vector<int>> mv{{1, 2, 3}, {4, 5, 6}};
   join_iterator it({mv.end(), mv.end()}, {mv.back().end(), mv.back().end()});
   join_iterator const cp = it;
   ++it;
-  EXPECT_THAT(it, cp);
+  EXPECT_NE(it, cp);
 }