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

explain why the side effects of peek matter #33269

Closed
tshepang opened this issue Apr 29, 2016 · 11 comments
Closed

explain why the side effects of peek matter #33269

tshepang opened this issue Apr 29, 2016 · 11 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools P-medium Medium priority

Comments

@tshepang
Copy link
Member

Why does one have to be aware of the side effects? See #33016 for the change.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Apr 29, 2016

It is said:

Note that the underlying iterator is still advanced when peek is
called for the first time: In order to retrieve the next element,
next is called on the underlying iterator, hence any side effects of
the next method will occur.

Doesn't that sound explicit enough?

@tshepang
Copy link
Member Author

tshepang commented Apr 29, 2016

Why do the side effects matter to the user of the API? What is it that they must watch out for that could affect them negatively?

@TyOverby
Copy link
Contributor

@tshepang If your iterator does things that have side-effects when next is called, then peek can trigger those side-effects. Similarly, after a peek, calling next won't trigger side-effects because the next value has already been cached.

This isn't an issue for most iterators, but if you are using iterators as an abstraction over a state machine, then the behavior could be confusing.

@tshepang
Copy link
Member Author

So this is about custom iterators, not iterators in the stdlib?

@TyOverby
Copy link
Contributor

TyOverby commented May 2, 2016

I don't think that there are any really weird iterators in the stdlib, but you can make them using the stdlib very easily.

let new_iter = my_vector.iter().map(|a| { println!("hi"); a });

Now you have an iterator with a side-effect that will behave weirdly when peeked at.

@tshepang
Copy link
Member Author

tshepang commented May 2, 2016

Oh, so side effect means doing stuff other than fetch the next value, that being writing to stdout in your example?

@arielb1
Copy link
Contributor

arielb1 commented May 2, 2016

That's what "side effect" means in this context - the effects of calling a function other than its value being returned.

@abonander
Copy link
Contributor

abonander commented May 2, 2016

Unexpected side-effects in the stdlib might include blocking and early consumption of values from channel and I/O iterators.

@steveklabnik steveklabnik added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Mar 10, 2017
@steveklabnik steveklabnik added P-medium Medium priority and removed A-docs labels Mar 22, 2017
@steveklabnik
Copy link
Member

It sounds like there might be confusion about the term "side effect" here, which I don't think is fixable in our docs. But maybe being more explicit could help.

@tshepang how are you feeling about this issue? Do you have any suggestions as to how this could be improved, now that you see what's going on?

@tshepang
Copy link
Member Author

tshepang commented Apr 25, 2017

We can add a bit to the text, e.g.:

Note that the underlying iterator is still advanced when peek is
called for the first time: In order to retrieve the next element,
next is called on the underlying iterator, hence any side effects (i.e.
anything other than just fetching the next value) of
the next method will occur.

If that's not satisfactory, I would not mind if this issue was closed.

@steveklabnik
Copy link
Member

Sounds good to me!

steveklabnik added a commit to steveklabnik/rust that referenced this issue Apr 25, 2017
arielb1 pushed a commit to arielb1/rust that referenced this issue Apr 25, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants