Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add itertools::generate #299

Closed
wants to merge 1 commit into from
Closed

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Aug 12, 2018

closes #298

No tests yet, will add them if we indeed want this in itertools :-)

@matklad
Copy link
Contributor Author

matklad commented Aug 15, 2018

Here's another example where generate simplifies stuff: rust-lang/rust-analyzer@a7d31b5?diff=split#diff-32acdb3e3f79ef45c2a6a0569d3812ddL16

I think I could've coded the original version better, by using Option<CurrentState>.take() pattern instead of that awkward bool, but it is hard to invent when you try to actually traverse that tree with parent pointers in O(1) stack space :)

@bluss
Copy link
Member

bluss commented Aug 26, 2018

thanks this looks very nice. The difference vs unfold makes sense. Maybe we can call out the difference more explicitly, or that the seed value is produced as the first value at least?

Why the choice to make the seed input parameter an Option? That seems to be the awkward concession with this API. I wonder if we can make it a plain value, and then the resulting iterator is always at least a sequence of one element.

@matklad
Copy link
Contributor Author

matklad commented Aug 26, 2018

seed being optional feels canonical: it allows for empty sequences and exactly matches the repr of the oterator. In practice, it is indeed the case that non-empty case is more common, but empty one is occasionally useful as well. Kotlin handles this gracefully because ? turns out to be more ergonomic than option in this case.

I think a way forward is to remove option here, and than maybe make the Generate::new public factory function which accepts an Option?

It's an easier to use alternative to unfoled,
modeled after Kotlin's generateSequence.
@matklad
Copy link
Contributor Author

matklad commented Aug 26, 2018

Pushed up update & some test :-)

@matklad
Copy link
Contributor Author

matklad commented Aug 27, 2018

@Emerentius
Copy link
Contributor

Emerentius commented Sep 7, 2018

I've defined a similar function for myself a couple of times like this:

fn generator<T>(mut advance: impl FnMut() -> Option<T>) -> impl Iterator<Item = T> {
    std::iter::repeat(()).scan((), move |(), ()| advance())
}

Which relies completely on closure captures. I think you don't really need a seed for generality. It may simplify the closure, but it forces a check that may be impossible to fail (infinite iterator) .

@matklad
Copy link
Contributor Author

matklad commented Nov 13, 2018

friendly ping @bluss :) Saw you on both internals and GitHub today ;)

@matklad
Copy link
Contributor Author

matklad commented Nov 13, 2018

CI is failing due to #302 , IIRC

@matklad
Copy link
Contributor Author

matklad commented Nov 24, 2018

std now has unstable successors function, so this can be closed now!

@matklad matklad closed this Nov 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unfold API seems less than ideal
3 participants