Repository: Peternator7/strum
Pull request #60 (2019-07-16 03:10:44)

Implemented DoubleEndedIterator for EnumIter.
This is a first working implementation of #59. 
All the tests pass including a new one to test the `DoubleEndedIterator`.

I have never used Rust macros before, so please tell me if something could be implemented better or something is missing.

Maybe it would be more elegant (and performant?) to generate a `static` or `const` array instead of the match arms and just return the arrays iterator in `iter`.

Peternator7: Simply using an iterator to an array is an interesting option. It would require an additional `Clone` bound on the enum though.

Iterators to arrays return references to the items in the array. EnumIter returns values which means we would need to clone the values returned by the array's iterator to pass them back to the user, and that requires the type be `Clone`.

You can definitely look at that code to see how they avoid overlapping forward and backward iteration though!
Nevsor: Thank you for the feedback! I did not notice the intended overlapping behaviour on my first reading of the DoubleEndedIterator trait. Unfortunately I will be offline for the rest of the week and will not be able to fix this before tuesday.
Peternator7: @Nevsor, are you still working on this?
Nevsor: @Peternator7, sorry for the delay. Here is my implementation of `DoubleEndedIterator`. I ended up implementing `next()` and `next_back()` as special cases of a new, more efficient implementation of `nth()` and `nth_back()`. 

dbc7a257b8cbe4aa98a38a7556fc3fb1fc398649 is not strictly related to this Pull Request, should it be merged seperately?

Patch:
diff --git a/strum_macros/src/macros/enum_iter.rs b/strum_macros/src/macros/enum_iter.rs
index a62bfd8..4d58985 100644
--- a/strum_macros/src/macros/enum_iter.rs
+++ b/strum_macros/src/macros/enum_iter.rs
@@ -62,14 +62,24 @@ pub fn enum_iter_inner(ast: &syn::DeriveInput) -> TokenStream {
         #[allow(missing_docs)]
         #vis struct #iter_name #ty_generics {
             idx: usize,
+            back_idx: usize,
             marker: ::std::marker::PhantomData #phantom_data,
         }
 
+        impl #impl_generics #iter_name #ty_generics #where_clause {
+            fn get(&self, idx: usize) -> Option<#name #ty_generics> {
+                match idx {
+                    #(#arms),*
+                }
+            }
+        }
+
         impl #impl_generics ::strum::IntoEnumIterator for #name #ty_generics #where_clause {
             type Iterator = #iter_name #ty_generics;
             fn iter() -> #iter_name #ty_generics {
                 #iter_name {
-                    idx:0,
+                    idx: 0,
+                    back_idx: 0,
                     marker: ::std::marker::PhantomData,
                 }
             }
@@ -78,21 +88,24 @@ pub fn enum_iter_inner(ast: &syn::DeriveInput) -> TokenStream {
         impl #impl_generics Iterator for #iter_name #ty_generics #where_clause {
             type Item = #name #ty_generics;
 
-            fn next(&mut self) -> Option<#name #ty_generics> {
-                let output = match self.idx {
-                    #(#arms),*
-                };
-
-                if self.idx < #variant_count {
-                    self.idx += 1;
-                }
-                output
+            fn next(&mut self) -> Option<Self::Item> {
+                self.nth(0)
             }
-
+            
             fn size_hint(&self) -> (usize, Option<usize>) {
-                let t = if self.idx >= #variant_count { 0 } else { #variant_count - self.idx };
+                let t = if self.idx + self.back_idx >= #variant_count { 0 } else { #variant_count - self.idx - self.back_idx };
                 (t, Some(t))
             }
+
+            fn nth(&mut self, n: usize) -> Option<Self::Item> {
+                self.idx += n + 1;
+
+                if self.idx + self.back_idx > #variant_count {
+                    None
+                } else {
+                    self.get(self.idx - 1)
+                }   
+            }
         }
 
         impl #impl_generics ExactSizeIterator for #iter_name #ty_generics #where_clause {
@@ -101,10 +114,27 @@ pub fn enum_iter_inner(ast: &syn::DeriveInput) -> TokenStream {
             }
         }
 
+        impl #impl_generics DoubleEndedIterator for #iter_name #ty_generics #where_clause {
+            fn next_back(&mut self) -> Option<Self::Item> {
+                self.nth_back(0)
+            }
+
+            fn nth_back(&mut self, n: usize) -> Option<Self::Item> {
+                self.back_idx += n + 1;
+
+                if self.idx + self.back_idx > #variant_count {
+                    None
+                } else {
+                    self.get(#variant_count - self.back_idx)
+                }  
+            }
+        }
+
         impl #impl_generics Clone for #iter_name #ty_generics #where_clause {
             fn clone(&self) -> #iter_name #ty_generics {
                 #iter_name {
                     idx: self.idx,
+                    back_idx: self.back_idx,
                     marker: self.marker.clone(),
                 }
             }


Test patch:
diff --git a/strum_tests/tests/enum_iter.rs b/strum_tests/tests/enum_iter.rs
index b0813ef..6e1975e 100644
--- a/strum_tests/tests/enum_iter.rs
+++ b/strum_tests/tests/enum_iter.rs
@@ -68,6 +68,21 @@ fn len_test() {
     assert_eq!(0, i.size_hint().1.unwrap());
 }
 
+#[test]
+fn double_ended_len_test() {
+    let mut i = Complicated::<(), ()>::iter();
+    assert_eq!(3, i.len());
+    i.next_back();
+
+    assert_eq!(2, i.len());
+    i.next();
+
+    assert_eq!(1, i.len());
+    i.next_back();
+
+    assert_eq!(0, i.len());
+}
+
 #[test]
 fn clone_test() {
     let mut i = Week::iter();
@@ -103,3 +118,60 @@ fn cycle_test() {
     ];
     assert_eq!(expected, results);
 }
+
+#[test]
+fn reverse_test() {
+    let results = Week::iter().rev().collect::<Vec<_>>();
+    let expected = vec![
+        Week::Saturday,
+        Week::Friday,
+        Week::Thursday,
+        Week::Wednesday,
+        Week::Tuesday,
+        Week::Monday,
+        Week::Sunday,
+    ];
+    assert_eq!(expected, results);
+}
+
+#[test]
+fn take_from_both_sides_test() {
+    let mut iter = Week::iter();
+
+    assert_eq!(Some(Week::Sunday), iter.next());
+    assert_eq!(Some(Week::Saturday), iter.next_back());
+    assert_eq!(Some(Week::Friday), iter.next_back());
+    assert_eq!(Some(Week::Monday), iter.next());
+    assert_eq!(Some(Week::Tuesday), iter.next());
+    assert_eq!(Some(Week::Wednesday), iter.next());
+    assert_eq!(Some(Week::Thursday), iter.next_back());
+    assert_eq!(None, iter.next());
+    assert_eq!(None, iter.next_back());
+}
+
+#[test]
+fn take_from_both_sides_test2() {
+    let mut iter = Week::iter();
+
+    assert_eq!(Some(Week::Sunday), iter.next());
+    assert_eq!(Some(Week::Saturday), iter.next_back());
+    assert_eq!(Some(Week::Friday), iter.next_back());
+    assert_eq!(Some(Week::Monday), iter.next());
+    assert_eq!(Some(Week::Tuesday), iter.next());
+    assert_eq!(Some(Week::Wednesday), iter.next());
+    assert_eq!(Some(Week::Thursday), iter.next());
+    assert_eq!(None, iter.next_back());
+    assert_eq!(None, iter.next());
+}
+
+#[test]
+fn take_nth_test() {
+    let mut iter = Week::iter();
+
+    assert_eq!(Some(Week::Tuesday), iter.nth(2));
+    assert_eq!(Some(Week::Saturday), iter.nth_back(0));
+    assert_eq!(Some(Week::Thursday), iter.nth_back(1));
+    assert_eq!(None, iter.nth(1));
+    assert_eq!(None, iter.next());
+    assert_eq!(None, iter.next_back());
+}
\ No newline at end of file