Skip to content

clarifying iterator trait documentation #15897

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

Closed
wants to merge 1 commit into from

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Jul 22, 2014

I found these things to be ambiguous, or at least worth stating explicitly to reduce the amount a user/developer needs to think about the API.

@@ -100,6 +100,7 @@ pub trait Iterator<A> {
fn next(&mut self) -> Option<A>;

/// Return a lower bound and upper bound on the remaining length of the iterator.
/// In the case that a valid upper bound cannot fit within a uint, None should be returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"None" should use backticks, e.g. None. Also, this documentation here is primarily aimed at the user instead of the implementor. This could be rephrased as

Returns a lower and upper bound on the remaining length of the iterator.
An upper bound of None means either there is no known upper bound, or the upper bound
does not fit within a uint.

@Gankra
Copy link
Contributor Author

Gankra commented Jul 22, 2014

@kballard: How's it look now?

pub trait RandomAccessIterator<A>: Iterator<A> {
/// Return the number of indexable elements. At most `std::uint::MAX`
/// elements are indexable, even if the iterator represents a longer range.
fn indexable(&self) -> uint;

/// Return an element at an index
/// Return an element at an index, or None if the index is out of bounds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None needs backticks, as in None

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

God damn, I double checked that I got them all and everything. I suck. Fixed.

@lilyball
Copy link
Contributor

LGTM. r=me if you squash all your commits together.

@Gankra
Copy link
Contributor Author

Gankra commented Jul 22, 2014

@kballard commits squashed.

bors added a commit that referenced this pull request Jul 23, 2014
I found these things to be ambiguous, or at least worth stating explicitly to reduce the amount a user/developer needs to think about the API.
@bors bors closed this Jul 23, 2014
@Gankra Gankra deleted the it-docs branch August 18, 2014 19:39
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.

4 participants