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.

Sign up for GitHub

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

doc: improve Drain examples #30272

Merged
merged 1 commit into from
Dec 18, 2015
Merged

doc: improve Drain examples #30272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions src/libcollections/string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,10 @@ impl String {
}

/// Create a draining iterator that removes the specified range in the string
/// and yields the removed chars from start to end. The element range is
/// removed even if the iterator is not consumed until the end.
/// and yields the removed chars.
///
Copy link
Member

Choose a reason for hiding this comment

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

The information about removal is good I think. You might otherwise think that only elements extracted from the iterator are removed from the String or Vec, or VecDeque.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bluss it's only the extracted elements that are removed

Copy link
Member

Choose a reason for hiding this comment

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

No it's not, I implemented this for Vec and String.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand you. Following does not panic:

let mut v = vec![1, 2, 3];
assert_eq!(vec![3], v.drain(2..).collect::<Vec<_>>());
assert_eq!(vec![1, 2], v);
v.drain(..);
assert!(v.is_empty());

Copy link
Member

Choose a reason for hiding this comment

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

ok, I mean that:

let mut v = vec![1, 2, 3];
v.drain(..).take(1).collect::<Vec<_>>();

All elements in the range are removed even if only 1 element is taken ("extracted") from the iterator.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now I understand. But that is somewhat orthogonal isn't it... not worth mentioning in the summary. Also, the examples I added also demonstrate the fact you mention.

Copy link
Member

Choose a reason for hiding this comment

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

It should be mentioned somewhere in the docs for the method IMO. It's something users have already wondered about, always best to say it explicitly.

Copy link
Member Author

Choose a reason for hiding this comment

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

alright

Copy link
Member Author

Choose a reason for hiding this comment

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

I re-instated the info @bluss

/// Note: The element range is removed even if the iterator is not
/// consumed until the end.
///
/// # Panics
///
Expand Down
17 changes: 11 additions & 6 deletions src/libcollections/vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,10 +725,12 @@ impl<T> Vec<T> {
}

/// Create a draining iterator that removes the specified range in the vector
/// and yields the removed items from start to end. The element range is
/// removed even if the iterator is not consumed until the end.
/// and yields the removed items.
///
/// Note: It is unspecified how many elements are removed from the vector,
/// Note 1: The element range is removed even if the iterator is not
/// consumed until the end.
///
/// Note 2: It is unspecified how many elements are removed from the vector,
/// if the `Drain` value is leaked.
///
/// # Panics
Expand All @@ -739,11 +741,14 @@ impl<T> Vec<T> {
/// # Examples
///
/// ```
/// // Draining using `..` clears the whole vector.
/// let mut v = vec![1, 2, 3];
/// let u: Vec<_> = v.drain(..).collect();
/// let u: Vec<_> = v.drain(1..).collect();
/// assert_eq!(v, &[1]);
/// assert_eq!(u, &[2, 3]);
///
/// // A full range clears the vector
/// v.drain(..);
/// assert_eq!(v, &[]);
/// assert_eq!(u, &[1, 2, 3]);
/// ```
#[stable(feature = "drain", since = "1.6.0")]
pub fn drain<R>(&mut self, range: R) -> Drain<T>
Expand Down
18 changes: 11 additions & 7 deletions src/libcollections/vec_deque.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,10 +763,12 @@ impl<T> VecDeque<T> {
}

/// Create a draining iterator that removes the specified range in the
/// `VecDeque` and yields the removed items from start to end. The element
/// range is removed even if the iterator is not consumed until the end.
/// `VecDeque` and yields the removed items.
///
/// Note: It is unspecified how many elements are removed from the deque,
/// Note 1: The element range is removed even if the iterator is not
/// consumed until the end.
///
/// Note 2: It is unspecified how many elements are removed from the deque,
/// if the `Drain` value is not dropped, but the borrow it holds expires
/// (eg. due to mem::forget).
///
Expand All @@ -779,11 +781,13 @@ impl<T> VecDeque<T> {
///
/// ```
/// use std::collections::VecDeque;

/// let mut v: VecDeque<_> = vec![1, 2, 3].into_iter().collect();
/// assert_eq!(vec![3].into_iter().collect::<VecDeque<_>>(), v.drain(2..).collect());
/// assert_eq!(vec![1, 2].into_iter().collect::<VecDeque<_>>(), v);
///
/// // draining using `..` clears the whole deque.
/// let mut v = VecDeque::new();
/// v.push_back(1);
/// assert_eq!(v.drain(..).next(), Some(1));
/// // A full range clears all contents
/// v.drain(..);
/// assert!(v.is_empty());
/// ```
#[inline]
Expand Down