let iterator facade use only public functions; declare compound and difference operators in derived iterator implementation

Signed-off-by: Martijn Govers <[email protected]>

Author: mgovers

power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/dataset.hpp

@@ -118,8 +118,6 @@ class ColumnarAttributeRange : public std::ranges::view_interface<ColumnarAttrib
     };
 
     class iterator : public IteratorFacade {
-        friend class IteratorFacade;
-
       public:
         using value_type = std::conditional_t<is_data_mutable_v<dataset_type>, Proxy, Proxy const>;
         using difference_type = Idx;
@@ -139,15 +137,19 @@ class ColumnarAttributeRange : public std::ranges::view_interface<ColumnarAttrib
             return first.current_idx() <=> second.current_idx();
         }
 
+        constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<iterator> {
+            current_idx() += n;
+            return *this;
+        }
+
+        friend constexpr auto operator-(iterator const& first, iterator const& second) -> difference_type {
+            return first.current_idx() - second.current_idx();
+        }
+
       private:
         constexpr auto current_idx() const { return current_.idx_; }
         constexpr auto& current_idx() { return current_.idx_; }
 
-        constexpr auto distance_to(iterator const& other) const -> difference_type {
-            return other.current_idx() - current_idx();
-        }
-        constexpr void advance(difference_type n) { current_idx() += n; }
-
         Proxy current_;
     };
 

power_grid_model_c/power_grid_model/include/power_grid_model/common/grouped_index_vector.hpp

@@ -95,8 +95,6 @@ constexpr auto from_dense = from_dense_t{};
 class SparseGroupedIdxVector {
   private:
     class GroupIterator : public IteratorFacade {
-        friend class IteratorFacade;
-
       public:
         using iterator = GroupIterator;
         using const_iterator = std::add_const_t<GroupIterator>;
@@ -125,19 +123,23 @@ class SparseGroupedIdxVector {
             return first.group_ <=> second.group_;
         }
 
+        constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<GroupIterator> {
+            group_ += n;
+            return *this;
+        }
+
+        friend auto operator-(GroupIterator const& first, GroupIterator const& second) -> difference_type {
+            assert(first.indptr_ == second.indptr_);
+            return first.group_ - second.group_;
+        }
+
       private:
         IdxVector const* indptr_{};
         Idx group_{};
         mutable std::remove_const_t<value_type>
             latest_value_{}; // making this mutable allows us to delay out-of-bounds checks until
                              // dereferencing instead of update methods. Note that the value will be
                              // invalidated at first update
-
-        constexpr auto distance_to(GroupIterator const& other) const -> difference_type {
-            assert(indptr_ == other.indptr_);
-            return other.group_ - group_;
-        }
-        constexpr void advance(Idx n) { group_ += n; }
     };
 
     constexpr auto group_iterator(Idx group) const { return GroupIterator{indptr_, group}; }
@@ -175,8 +177,6 @@ class SparseGroupedIdxVector {
 class DenseGroupedIdxVector {
   private:
     class GroupIterator : public IteratorFacade {
-        friend class IteratorFacade;
-
       public:
         using iterator = GroupIterator;
         using const_iterator = std::add_const_t<GroupIterator>;
@@ -208,6 +208,15 @@ class DenseGroupedIdxVector {
             return first.group_ <=> second.group_;
         }
 
+        constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<GroupIterator> {
+            advance(n);
+            return *this;
+        }
+        friend auto operator-(GroupIterator const& first, GroupIterator const& second) -> difference_type {
+            assert(first.dense_vector_ == second.dense_vector_);
+            return first.group_ - second.group_;
+        }
+
       private:
         using group_iterator = IdxVector::const_iterator;
 
@@ -219,10 +228,6 @@ class DenseGroupedIdxVector {
                              // dereferencing instead of update methods. Note that the value will be
                              // invalidated at first update
 
-        constexpr auto distance_to(GroupIterator const& other) const -> difference_type {
-            return other.group_ - group_;
-        }
-
         constexpr void increment() {
             ++group_;
             group_range_ = {group_range_.end(), std::find_if(group_range_.end(), std::cend(*dense_vector_),
@@ -235,7 +240,7 @@ class DenseGroupedIdxVector {
                                 .base(),
                             group_range_.begin()};
         }
-        constexpr void advance(Idx n) {
+        constexpr void advance(difference_type n) {
             auto const start = n > 0 ? group_range_.end() : std::cbegin(*dense_vector_);
             auto const stop = n < 0 ? group_range_.begin() : std::cend(*dense_vector_);
 

power_grid_model_c/power_grid_model/include/power_grid_model/common/iterator_facade.hpp

@@ -13,17 +13,22 @@
 namespace power_grid_model {
 namespace detail {
 template <typename T>
-concept iterator_facadeable_c =
-    std::integral<typename T::difference_type> && std::is_pointer_v<typename T::pointer> &&
-    std::is_lvalue_reference_v<typename T::reference> &&
-    requires(T t, std::add_const_t<T> ct, std::add_const_t<T> ct2, typename T::difference_type d) {
-        typename T::value_type;
-        { *t } -> std::same_as<typename T::reference>;
-        { &*t } -> std::same_as<typename T::pointer>;
-        { ct <=> ct2 } -> std::same_as<std::strong_ordering>;
-        { t.advance(d) };
-        { ct.distance_to(ct) } -> std::same_as<typename T::difference_type>;
-    };
+concept iterator_facadeable_c = std::integral<typename T::difference_type> && std::is_pointer_v<typename T::pointer> &&
+                                std::is_lvalue_reference_v<typename T::reference> &&
+                                requires(T t, std::remove_cvref_t<T> mt, std::add_const_t<T> ct,
+                                         std::add_const_t<T> ct2, typename std::remove_cvref_t<T>::difference_type d) {
+                                    typename T::value_type;
+                                    { *t } -> std::same_as<typename T::reference>;
+                                    { &*t } -> std::same_as<typename T::pointer>;
+                                    { ct <=> ct2 } -> std::same_as<std::strong_ordering>;
+                                    { mt += d } -> std::same_as<std::add_lvalue_reference_t<T>>;
+                                    { ct - ct2 } -> std::same_as<typename T::difference_type>;
+                                };
+
+template <typename T, typename Int>
+concept compound_assignable_c = requires(T t, Int n) {
+    { t += n } -> std::same_as<std::add_lvalue_reference_t<T>>;
+};
 } // namespace detail
 
 class IteratorFacade {
@@ -71,43 +76,29 @@ class IteratorFacade {
         return result;
     }
     template <typename Self>
-    constexpr std::add_lvalue_reference_t<Self> operator+=(this Self& self, std::integral auto offset) {
-        self.advance(offset);
-        return self;
-    }
-    template <typename Self>
     constexpr std::add_lvalue_reference_t<Self> operator-=(this Self& self, std::integral auto idx) {
         return (self += (-idx));
     }
 
-    template <typename Self>
-        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> &&
-                 detail::iterator_facadeable_c<std::remove_cvref_t<Self>>
-    friend constexpr std::remove_cvref_t<Self> operator+(Self&& self, std::integral auto offset) {
+    template <typename Self, std::integral Int>
+    friend constexpr std::remove_cvref_t<Self> operator+(Self&& self, Int offset)
+        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
+    {
         using Result = std::remove_cvref_t<Self>;
         Result result{std::forward<Self>(self)};
         result += offset;
         return result;
     }
-    template <typename Self>
-        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> &&
-                 detail::iterator_facadeable_c<std::remove_cvref_t<Self>>
-    friend constexpr std::remove_cvref_t<Self> operator+(std::integral auto offset, Self&& self) {
+    template <typename Self, std::integral Int>
+        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
+    friend constexpr std::remove_cvref_t<Self> operator+(Int offset, Self&& self) {
         return std::forward<Self>(self) + offset;
     }
-    template <typename Self>
-        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> &&
-                 detail::iterator_facadeable_c<std::remove_cvref_t<Self>>
-    friend constexpr std::remove_cvref_t<Self> operator-(Self&& self, std::integral auto idx) {
+    template <typename Self, std::integral Int>
+        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> //&& detail::iterator_facadeable_c<Self>
+    friend constexpr std::remove_cvref_t<Self> operator-(Self&& self, Int idx) {
         return (std::forward<Self>(self)) + (-idx);
     }
-    template <typename Self, typename Other>
-        requires std::derived_from<std::remove_cvref_t<Self>, IteratorFacade> &&
-                 detail::iterator_facadeable_c<std::remove_cvref_t<Self>> &&
-                 std::same_as<std::remove_cvref_t<Self>, std::remove_cvref_t<Other>>
-    friend constexpr auto operator-(Self&& self, Other&& other) {
-        return std::forward<Other>(other).distance_to(std::forward<Self>(self));
-    }
 
     template <typename Self>
     constexpr decltype(auto) operator[](this Self const& self, typename Self::difference_type idx) {
@@ -122,7 +113,7 @@ class IteratorFacade {
         // instantiated when used.
         static_assert(std::derived_from<std::remove_cvref_t<Self>, IteratorFacade>);
         static_assert(detail::iterator_facadeable_c<std::remove_cvref_t<Self>>);
-    };
+    }
 };
 
 } // namespace power_grid_model

power_grid_model_c/power_grid_model/include/power_grid_model/container.hpp

@@ -286,8 +286,6 @@ class Container<RetrievableTypes<GettableTypes...>, StorageableTypes...> {
 
     // define iterator
     template <supported_type_c<GettableTypes...> Gettable> class Iterator : public IteratorFacade {
-        friend class IteratorFacade;
-
       public:
         using value_type = Gettable;
         using difference_type = Idx;
@@ -312,17 +310,22 @@ class Container<RetrievableTypes<GettableTypes...>, StorageableTypes...> {
         }
         constexpr Gettable& operator*() { return container_ptr_->template get_item_by_seq<base_type>(idx_); }
 
-        friend constexpr auto operator<=>(Iterator const& first, Iterator const& second) {
+        constexpr auto operator+=(Idx n) -> std::add_lvalue_reference_t<Iterator> {
+            idx_ += n;
+            return *this;
+        }
+
+        friend constexpr auto operator<=>(Iterator const& first, Iterator const& second) -> std::strong_ordering {
             assert(first.container_ptr_ == second.container_ptr_);
             return first.idx_ <=> second.idx_;
         }
+        friend constexpr auto operator-(Iterator const& first, Iterator const& second) -> difference_type {
+            assert(first.container_ptr_ == second.container_ptr_);
+            return first.idx_ - second.idx_;
+        }
 
       private:
         constexpr void advance(Idx n) { idx_ += n; }
-        constexpr Idx distance_to(Iterator const& other) const {
-            assert(container_ptr_ == other.container_ptr_);
-            return other.idx_ - idx_;
-        }
 
         // store container pointer
         // and idx

tests/cpp_unit_tests/test_iterator_facade.cpp

@@ -15,8 +15,6 @@ using power_grid_model::IteratorFacade;
 using power_grid_model::detail::iterator_facadeable_c;
 
 template <typename UnderlyingType> class BaseTestIterator : public IteratorFacade {
-    friend class IteratorFacade;
-
   public:
     using underlying = UnderlyingType;
 
@@ -35,8 +33,13 @@ template <typename UnderlyingType> class BaseTestIterator : public IteratorFacad
         return *first.it_ <=> *second.it_;
     }
 
-    constexpr auto distance_to(BaseTestIterator const& other) const -> difference_type { return other.it_ - it_; }
-    constexpr void advance(difference_type n) { it_ += n; }
+    constexpr auto operator+=(difference_type n) -> std::add_lvalue_reference_t<BaseTestIterator> {
+        it_ += n;
+        return *this;
+    }
+    friend constexpr auto operator-(BaseTestIterator const& first, BaseTestIterator const& second) -> difference_type {
+        return second.it_ - first.it_;
+    }
 
   private:
     underlying it_;