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 workings of successors more clearly #135895

Merged
merged 1 commit into from
Mar 7, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions library/core/src/iter/sources/successors.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use crate::fmt;
use crate::iter::FusedIterator;

/// Creates a new iterator where each successive item is computed based on the preceding one.
/// Creates an iterator which, starting from an initial item,
/// computes each successive item from the preceding one.
///
/// The iterator starts with the given first item (if any)
/// and calls the given `FnMut(&T) -> Option<T>` closure to compute each item’s successor.
/// The iterator will yield the `T`s returned from the closure.
/// This iterator stores an optional item (`Option<T>`) and a successor closure (`impl FnMut(&T) -> Option<T>`).
/// Its `next` method returns the stored optional item and
/// if it is `Some(val)` calls the stored closure on `&val` to compute and store its successor.
/// The iterator will apply the closure successively to the stored option's value until the option is `None`.
/// This also means that once the stored option is `None` it will remain `None`,
/// as the closure will not be called again, so the created iterator is a [`FusedIterator`].
/// The iterator's items will be the initial item and all of its successors as calculated by the successor closure.
Copy link
Member

Choose a reason for hiding this comment

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

This seems redundant to me – isn't this explained by the preceding sentences?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is mostly there as a direct result of the old text saying "The iterator will yield the Ts returned from the closure.", and it does read pretty similar to the topline text ("Creates an iterator which, starting from an initial item, computes each successive item from the preceding one."), but includes slightly more detail (mentioning the successor closure which was explained in the previous paragraph.

I think it's still useful as a nice summary of the previous paragraph, which is quite detailed, and may be more readily understood than the detailed paragraph. But if you still think it is better without it, then I can remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough...

///
/// ```
/// use std::iter::successors;
Expand All @@ -24,7 +29,8 @@ where
Successors { next: first, succ }
}

/// A new iterator where each successive item is computed based on the preceding one.
/// An iterator which, starting from an initial item,
/// computes each successive item from the preceding one.
///
/// This `struct` is created by the [`iter::successors()`] function.
/// See its documentation for more.
Expand Down
Loading