Add ArrayVec::resize and friends
#82
Conversation
`ArrayVec::resize_default` `ArrayVec::resize` `ArrayVec::try_resize_default` `ArrayVec::try_resize` Fixes bluss#72.
|
This duplicates the set Len on drop-ish struct. It can be refactored, can't it? |
|
Thanks a lot for working on this. To clarify from before, it looks like this duplicates instead of factors out the logic inside extend and its ScopeExitGuard (which has the same function as SetLenOnDrop). It's best to not duplicate it. What's the reason for the extra methods and types? Maybe the existing |
This removes the use of `ScopeExitGuard`.
|
@bluss I basically copied it from stdlib's implementation. |
src/lib.rs
Outdated
| @@ -597,6 +597,190 @@ impl<A: Array> ArrayVec<A> { | |||
| } | |||
| } | |||
|
|
|||
| impl<A: Array> ArrayVec<A> | |||
| where A::Item: Clone, | |||
| { | |||
There was a problem hiding this comment.
Style wise I'd prefer a single impl block. The where clause can go on each method fine. Split impl blocks feels archaic.
src/lib.rs
Outdated
| /// Resizes the `ArrayVec` in-place so that `len` is equal to `new_len`. | ||
| /// | ||
| /// Does the same thing as `try_resize`, but panics on error instead of | ||
| /// returning it as a `Result`. |
There was a problem hiding this comment.
Documentation for panic should be consistent and visible. Other methods use this style, panic the first word of the line and bold:
/// ***Panics*** if the `index` is out of bounds.
src/lib.rs
Outdated
| self.try_resize(new_len, value).unwrap() | ||
| } | ||
|
|
||
| /// Resizes the `ArrayVec` in-place so that `len` is equal to `new_len`. |
There was a problem hiding this comment.
Rust's own formulation in their docs "so that len is equal" is awkward. Sounds better to just say "so that its length becomes new_len" or similar.
src/lib.rs
Outdated
|
|
||
| /// Resizes the `ArrayVec` in-place so that `len` is equal to `new_len`. | ||
| /// | ||
| /// If `new_len` is greater than `len`, the `ArrayVec` is extended by the |
There was a problem hiding this comment.
I think we should say length instead of len.
src/lib.rs
Outdated
| fn extend_with<E: ExtendWith<A::Item>>(&mut self, n: usize, value: E) | ||
| -> Result<(), CapacityError> | ||
| { | ||
| if self.len() + n > self.capacity() { |
There was a problem hiding this comment.
This addition feels wrong, as if it could wrap around. But it's a private function and all callers are without problem.
Also fix a couple of doc strings.
|
Done I think. |
|
Cool. Please have some patience. I'll make a new suggestion :-) |
There was a problem hiding this comment.
extend_with doesn't need to be removed -- it can simply be a method that takes the needed_capacity number as the first parameter and the iterator as the second. Do error handling and then pass on the iterator to the existing extend method. Do you think that works?
| /// | ||
| /// Doesn't extend the vector and returns an error if the number of added | ||
| /// elements exceed the capacity of the underlying array. | ||
| fn extend_with<E>(&mut self, values: E) -> Result<(), CapacityError> |
There was a problem hiding this comment.
This should just use the existing extend I think.
| Some(if self.0 > 0 { | ||
| self.1.as_ref().unwrap().clone() | ||
| } else { | ||
| self.1.take().unwrap() |
There was a problem hiding this comment.
These .unwrap are avoidable, aren't they? The Some/None-ness can be used to guard the end of the iteration.
There was a problem hiding this comment.
One could spy at itertools' repeat_n, but I'm not sure if that spoils the fun.
There was a problem hiding this comment.
or indeed if it spoils the performance.
| } | ||
| } | ||
|
|
||
| struct ExtendDefault<T: Default>(usize, PhantomData<T>); |
There was a problem hiding this comment.
The ExtendDefault iterator is not needed, it can be passed as a regular iterator inline, maybe something like (0..n).map(|_| X::default())
| } | ||
| } | ||
|
|
||
| struct ExtendIter<I: Iterator>(I); |
There was a problem hiding this comment.
This trivial wrapper is not needed.
|
I'll pick this up at some point |
ArrayVec::resize_defaultArrayVec::resizeArrayVec::try_resize_defaultArrayVec::try_resizeFixes #72.