Skip to content

unfold API seems less than ideal #298

New issue

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

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

Already on GitHub? # to your account

Closed
matklad opened this issue Aug 11, 2018 · 3 comments
Closed

unfold API seems less than ideal #298

matklad opened this issue Aug 11, 2018 · 3 comments

Comments

@matklad
Copy link
Contributor

matklad commented Aug 11, 2018

Hi! recently, I've been using unfold a couple of times, and looks like it's rather tricky, at least in comparison with Kotlin's generateSequece.

I see two problems with current unfold API:

  • There's two ways to maintain state: you can use the initial_state, or you can just close over some env (I think these are equivalent in power).

  • There'a common case when the state and the Item of the iterator are the same. In this situation, you need to be careful to update the state, but return a previous value.

It seems to me that, instead of unfold, ideally there should be a couple of function:

  • repeat_call, for explicitly closing over some state and handling updates.

  • generate<T>(first: Option<T>, step: impl FnMut(&T) -> Option<T>) -> impl Iterator<Item=T> for the common case.

Here's an example usage, which compares unfold and generate:

https://github.com/matklad/libsyntax2/blob/557feed21ab4f1a6473b5f79e6ef250cf1219857/crates/libeditor/src/code_actions.rs#L42-L53

The second one looks much clearer to me :)

Or am missing some property of generate, which makes it more powerful than repeat_call or easier to use than generate?

@matklad
Copy link
Contributor Author

matklad commented Aug 12, 2018

Send a PR with generate implementation. I've also noticed that the state field of Unfold is public, which indeed makes it a more general API. Perhas unfold's example should be changed to something which makes use of this feature?

@bluss
Copy link
Member

bluss commented Aug 26, 2018

You are right the current unfold example is just a fun switcheroo I did (at some point in the commit history), from explicit state to closure captures, but we can switch that right back. It's redundant to have both available, in a way.

@bluss
Copy link
Member

bluss commented Nov 24, 2018

This seems to be handled by std now. I'll add unfold to the potentially-to-remove list in itertools: #223

@bluss bluss closed this as completed Nov 24, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants