From fed56903dd5c9b59976711cf51c8af07754ed0a0 Mon Sep 17 00:00:00 2001
From: WiktorNowak <wnowak2f@gmail.com>
Date: Sat, 18 Jan 2025 13:24:11 +0100
Subject: [PATCH] fix: use peekable iterator to force interperse to work
 properly

---
 include/rusty_iterators/interperse.hpp | 35 +++++++++++++++-----------
 tests/interperse.test.cpp              | 15 ++++++++---
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/include/rusty_iterators/interperse.hpp b/include/rusty_iterators/interperse.hpp
index a556cec..569d584 100644
--- a/include/rusty_iterators/interperse.hpp
+++ b/include/rusty_iterators/interperse.hpp
@@ -1,7 +1,9 @@
 #pragma once
 
 #include "interface.fwd.hpp"
+#include "peekable.hpp"
 
+#include <algorithm>
 #include <optional>
 
 namespace rusty_iterators::iterator
@@ -13,35 +15,38 @@ class Interperse : public IterInterface<T, Interperse<T, Other>>
 {
   public:
     explicit Interperse(Other&& it, T&& item)
-        : it(std::forward<Other>(it)), item(std::forward<T>(item))
+        : it(std::forward<Peekable<T, Other>>(it.peekable())), item(std::forward<T>(item))
     {}
 
     auto next() -> std::optional<T>;
     [[nodiscard]] auto sizeHint() const -> std::optional<size_t>;
 
   private:
-    Other it;
+    Peekable<T, Other> it;
     T item;
-    bool interperse = false;
+    bool returnInterperseValue = false;
 };
 } // namespace rusty_iterators::iterator
 
 template <class T, class Other>
 auto rusty_iterators::iterator::Interperse<T, Other>::next() -> std::optional<T>
 {
-    if (interperse)
+    /// NOTE: 18.01.2025 <@uncommon-nickname>
+    /// We need to check if iterator is finished. By using a peekable
+    /// iterator, we can do that without any significant cost, because
+    /// we will advance the iterator anyway later.
+    auto peeked = it.peek();
+
+    [[unlikely]] if (!peeked.has_value())
+        return std::nullopt;
+
+    if (returnInterperseValue)
     {
-        interperse = false;
+        returnInterperseValue = false;
         return item;
     }
-
-    auto nextItem = it.next();
-
-    if (!nextItem.has_value())
-        return std::move(nextItem);
-
-    interperse = true;
-    return std::move(nextItem);
+    returnInterperseValue = true;
+    return std::move(it.next());
 }
 
 template <class T, class Other>
@@ -52,5 +57,7 @@ auto rusty_iterators::iterator::Interperse<T, Other>::sizeHint() const -> std::o
     if (!underlyingSize.has_value())
         return std::nullopt;
 
-    return underlyingSize.value() * 2;
+    int32_t potentialSize = (underlyingSize.value() * 2) - 1;
+
+    return std::max(0, potentialSize);
 }
diff --git a/tests/interperse.test.cpp b/tests/interperse.test.cpp
index b3993d5..419ce9f 100644
--- a/tests/interperse.test.cpp
+++ b/tests/interperse.test.cpp
@@ -16,7 +16,6 @@ TEST(TestInterperseIterator, TestNextReturnsInterpersed)
     ASSERT_EQ(it.next(), 1);
     ASSERT_EQ(it.next(), 5);
     ASSERT_EQ(it.next(), 2);
-    ASSERT_EQ(it.next(), 5);
     ASSERT_EQ(it.next(), std::nullopt);
 }
 
@@ -27,7 +26,7 @@ TEST(TestInterperseIterator, TestCollectReturnsAllValues)
     auto item = 5;
     auto it   = LazyIterator{vec}.interperse(std::cref(item));
 
-    EXPECT_THAT(it.collect(), ElementsAreArray({1, 5, 2, 5, 3, 5}));
+    EXPECT_THAT(it.collect(), ElementsAreArray({1, 5, 2, 5, 3}));
 }
 
 TEST(TestInterperseIterator, TestSizeHintIsDoubled)
@@ -37,5 +36,15 @@ TEST(TestInterperseIterator, TestSizeHintIsDoubled)
     auto item = 5;
     auto it   = LazyIterator{vec}.interperse(std::cref(item));
 
-    ASSERT_EQ(it.sizeHint(), 6);
+    ASSERT_EQ(it.sizeHint(), 5);
+}
+
+TEST(TestInterperseIterator, TestEmptyIteratorSizeIsZero)
+{
+    auto vec = std::vector<int>{};
+
+    auto item = 5;
+    auto it   = LazyIterator{vec}.interperse(std::cref(item));
+
+    ASSERT_EQ(it.sizeHint(), 0);
 }