Repository: rust-itertools/itertools (language: rust)

Issue:
PutBack behavior
Thank you for itertools!

It's my fault for not reading the documentation for `put_back`, but I accidentally [wrote a bug](https://github.com/casey/just/issues/363) by putting two items back into an iterator and overwriting the old one.

I might be wrong, but this behavior seems a bit error prone, and might be unexpected.

I would consider renaming `PutBack` to `PutBackOne`, and `PutBackN` to `PutBack`. Unless the overwriting behavior is desirable, `PutBack` is essentially a speed/memory optimization when compared to `PutBackN`, so I would guide users to what is currently `PutBackN`, and they can then switch to what is currently `PutBack` as an optimization.

If the overwriting behavior is never desirable, it might be good to make it panic on the second call to `put_back` in a row. In my case, this would have saved me a bit of debugging time.


Discussion / hints:
I see your point it might be error prone or unexpected (even if the documentation is clear enough).

I see your point about renaming but no other seems to bother this while such change would break everyone.

Panic on the "second call in a row" would be too much of a breaking change as people might be happy with the current behavior and would not have an easy alternative.

_I guess the `put_back` method could return the old value instead of just discarding it as it would at least indicates more what it is doing._
Still a breaking change but it would be reasonable.
@phimuemue Do you think `PutBack::put_back` should return the old item (instead of discarding it)?
> @phimuemue Do you think `PutBack::put_back` should return the old item (instead of discarding it)?

Came to my mind, too, when I first read the question.

Fix patch:
diff --git a/src/adaptors/mod.rs b/src/adaptors/mod.rs
index cf9642bc5..191d551da 100644
--- a/src/adaptors/mod.rs
+++ b/src/adaptors/mod.rs
@@ -280,10 +280,10 @@ where
 
     /// Put back a single value to the front of the iterator.
     ///
-    /// If a value is already in the put back slot, it is overwritten.
+    /// If a value is already in the put back slot, it is returned.
     #[inline]
-    pub fn put_back(&mut self, x: I::Item) {
-        self.top = Some(x);
+    pub fn put_back(&mut self, x: I::Item) -> Option<I::Item> {
+        self.top.replace(x)
     }
 }
 


Test patch:
diff --git a/tests/quick.rs b/tests/quick.rs
index b2982a6ef..e345c9813 100644
--- a/tests/quick.rs
+++ b/tests/quick.rs
@@ -882,7 +882,7 @@ quickcheck! {
     fn size_put_back(a: Vec<u8>, x: Option<u8>) -> bool {
         let mut it = put_back(a.into_iter());
         if let Some(t) = x {
-            it.put_back(t)
+            it.put_back(t);
         }
         correct_size_hint(it)
     }