Never implement `Iterator` for a collection!
ChayimFriedman2 opened this issue · 3 comments
Implement IntoIterator
instead. This is what allows for
loop, and should be implemented for iterables (As opposed to Iterator
which should be implemented for iterators).
Here is a rewrite of your example, including non-consuming iterators:
struct Metropolis {
cities: Vec<String>,
population: u32
}
impl IntoIterator for Metropolis {
type Item = String;
type IntoIter = IntoIter;
fn into_iter(self) -> Self::IntoIter {
IntoIter(self)
}
}
pub struct IntoIter(Metropolis);
impl Iterator for IntoIter {
type Item = String;
fn next(&mut self) -> Option<Self::Item> {
if !self.0.cities.is_empty() {
Some(self.0.cities.remove(0))
} else {
None
}
}
}
Yeah...the thing is though, this book is intended to be a simple easing into one language for users of the other (we're not trying to scare the Fsharpers away). Examples should be simple like the one in here:
https://doc.rust-lang.org/book/ch13-02-iterators.html
impl Iterator for Counter {
type Item = u32;
fn next(&mut self) -> Option<Self::Item> {
if self.count < 5 {
self.count += 1;
Some(self.count)
} else {
None
}
}
}
I don't think we can expect someone new to the language to implement IntoIterator with two associated types, followed by yet another implementation.
The same goes with the "".to_string() example elsewhere which is maybe not idiomatic but keep in mind that the reader has probably only just learned what the difference between a &str and a String is. Following that up with String::new() String::default() (and .into() or .to_owned() no less depending on the context!) is certainly frightening.
Although for that .to_string() example I think I can just rewrite it to avoid the "".to_string().
In .NET land there are also System.Collections.Generic.IEnumerable<'T>
(seq<'T>
) and System.Collections.Generic.IEnumerator<'T>
which seem to parallel this.
@ChayimFriedman2
Does this look good?
type Metropolis = {
cities: string ResizeArray
population: uint
} with
interface seq<string> with
member this.GetEnumerator() = new Iterator(this) :> _
interface System.Collections.IEnumerable with
member this.GetEnumerator() = new Iterator(this) :> _
and Iterator(metropolis) =
let mutable current = Unchecked.defaultof<_>
interface System.Collections.Generic.IEnumerator<string> with
member _.Current = current
member _.Dispose() = failwith "No equivalent"
interface System.Collections.IEnumerator with
member this.MoveNext() =
if metropolis.cities.Count > 0 then
current <- metropolis.cities.[0]
metropolis.cities.RemoveAt 0
true
else false
member _.Reset() = failwith "No equivalent"
member _.Current = current :> _