api-guidelines icon indicating copy to clipboard operation
api-guidelines copied to clipboard

Naming guidelines recommend `into_iter()` method instead of `IntoIterator` trait

Open Aloso opened this issue 5 years ago • 3 comments

C-ITER recommends implementing three methods for collections: iter(), iter_mut() and into_iter(). It fails to mention that into_iter() can (and probably should?) be implemented by implementing the IntoIterator trait.

Additionally, the Examples section has a broken link to https://doc.rust-lang.org/std/vec/struct.Vec.html#method.into_iter instead of https://doc.rust-lang.org/std/vec/struct.Vec.html#impl-IntoIterator.

Aloso avatar Nov 14 '19 23:11 Aloso

You don't technically need to use the IntoIterator trait in order to work with for loops (it just expects an into_iter method), but we don't actually appear to have any collections that use an inherent into_iter method. The closest is BinaryHeap::into_iter_sorted, which generated some discussion about naming in an ultimately closed PR to stabilize it. I think the conclusion we came to there was that additional iterator methods like into_iter_sorted should use the iter, iter_mut and into_iter prefixes.

KodrAus avatar Dec 21 '20 22:12 KodrAus

We should probably also mention implementing IntoIterator for &Self and &mut Self too that mirrors the inherent iter and iter_mut methods.

KodrAus avatar Dec 21 '20 22:12 KodrAus

You don't technically need to use the IntoIterator trait in order to work with for loops

@KodrAus that isn't true. For example:

struct Foo;

impl Foo {
    fn into_iter(self) -> Bar {
        Bar
    }
}

struct Bar;

impl Iterator for Bar {
    type Item = i32;

    fn next(&mut self) -> Option<i32> {
        Some(42)
    }
}

fn main() {
    for _ in Foo {
        break;
    }
}

This doesn't compile, because Foo is not an iterator. If the IntoIterator trait is implemented, however, it works.

P.S. I guess you meant that you can write for _ in Foo.into_iter() {}, but that's not very idiomatic.

Aloso avatar Dec 23 '20 12:12 Aloso