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

Document connection between Iterator::skip and Iterator::nth #60223

Closed
jhwgh1968 opened this issue Apr 24, 2019 · 11 comments · Fixed by #61237
Closed

Document connection between Iterator::skip and Iterator::nth #60223

jhwgh1968 opened this issue Apr 24, 2019 · 11 comments · Fixed by #61237
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-iterators Area: Iterators C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@jhwgh1968
Copy link

jhwgh1968 commented Apr 24, 2019

From my forum question:

Consider an iterator that does two things:

  1. Read data from a data source.
  2. Perform a complex calculation on the data read.

This means that next is computationally expensive, but skip could be cheap if
implemented by hand. It could just do step 1 and throw the data away without doing
step 2.

I have tried various ways to do this [... but] I cannot find a public API to create a Skip except by calling Iterator::skip.

Is this impossible?

It's easy, according to @scottmcm: implement my own Iterator::nth method, and I will get a skip implementation for free.

In retrospect it makes sense, but it was not obvious to me at all just from reading the docs. It is not mentioned in the Implementing Iterators section of the API, or the docs for either method. And as the link in my post suggests, trying to work my way around the ownership of the APIs led me quite far afield.

Admittedly it's not the most common case, but I think it would save others time if the standard library at least mentioned this somewhere.

EDIT: fixed forum link which changed its name when I marked it solved.

@scottmcm
Copy link
Member

A section about "you should consider overloading these things" definitely sounds good. fold should also be on that list. (In the future or in nightly try_fold is in that list too, as its docs mention.)

@jonas-schievink jonas-schievink added A-iterators Area: Iterators C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Apr 24, 2019
@Stargateur
Copy link
Contributor

Your forum link is broken.

@steveklabnik
Copy link
Member

The docs do say

Iterator's full definition includes a number of other methods as well, but they are default methods, built on top of next, and so you get them for free.

However, I do like @scottmcm 's suggestion, about talking about which methods you may want to override.

@the8472
Copy link
Member

the8472 commented Apr 24, 2019

Such hints should be non-normative though to allow the implementations, especially specializations, to use different approaches in the future.

@scottmcm
Copy link
Member

Agreed, @the8472. I was picturing more which methods to consider overloading, not specifically which things use them. (Or, contrapositively, which things there's usually no reason to overload, like for_each, because the default implementation is generally as good as it gets because it's a trivial wrapper.)

@DevQps
Copy link
Contributor

DevQps commented May 4, 2019

So basically this issue is about adding some documentation on which methods you might want to override right? Which methods do you think are best to mention? Or did I misinterpret the discussion?

@scottmcm
Copy link
Member

scottmcm commented May 4, 2019

@DevQps That's correctly, probably under or near https://doc.rust-lang.org/nightly/std/iter/index.html#implementing-iterator. For now my inclination would be to mention nth and fold -- once stable it should be try_fold instead, but IIRC we don't mention unstable in the broader docs until it stabilizes.

@DevQps
Copy link
Contributor

DevQps commented May 7, 2019

What about adding this line of text at the end of the "Implementing Iterator" section?

Also note that Iterator provides a default implementation of methods such as nth and fold
which call next internally. However, it is also possible to write a custom implementation of
methods such as nth and fold if an iterator is able to compute them more efficiently without calling
next.

Feel free to adjust the text or make something up yourself :) If you agree I will create a PR for this.

@czipperz
Copy link
Contributor

I think it would also be nice to add something to the skip documentation. How about this?

Rather than overriding this method directly, instead override the nth method.

@jhwgh1968
Copy link
Author

@DevQps, in case it wasn't clear or you are not watching your GitHub notifications, I approve of both your and @czipperz's suggestions. I look forward to the PR!

@DevQps
Copy link
Contributor

DevQps commented May 27, 2019

@jhwgh1968 Sorry for the late reply! I apparently don't get notifications for thumbs ups and the like! I will hopefully be able to create a PR somewhere this midday.

Centril added a commit to Centril/rust that referenced this issue May 28, 2019
…-Simulacrum

Updated the Iterator docs with information about overriding methods.

# Description

Updated the Iterator docs with information about overriding methods.

closes rust-lang#60223
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 A-iterators Area: Iterators C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants