Add BinaryHeap::retain#668
Conversation
87e788b to
64e77df
Compare
How so? |
The implementation doesn't preserve the order invariant of the binary heap. |
Huh? Is it? 🤔 https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=98dfdde25b2a102d7146761f6d5294db |
|
I will add some tests for |
Your example is wrong because you called |
Ah right. That was from the std doc example I started off from. |
4c41991 to
d8e3a5c
Compare
985fee8 to
beb1189
Compare
58678fe to
1535316
Compare
|
Ready for review! |
zeenix
left a comment
zeenix
left a comment
There was a problem hiding this comment.
Only tiny nits and then it's good from my side.
|
|
||
| - Added `resize_with` to `Vec` | ||
| - Added `retain_back` (aka `truncate_front`) to `Deque` | ||
| - Added `retain` to `BinaryHeap` |
There was a problem hiding this comment.
missing .. I know the previous entries did the same but let's not perpetuate this. :) Feel free to add to them too.
| /* Private API */ | ||
|
|
||
| /// Removes and returns the element at position `index` within the inner vec. | ||
| /// The elements are shifted to preserve the invariants of the binary heap. |
There was a problem hiding this comment.
Let's follow the Rust convention: title + empty line and then explanation etc in the next paragraph:
| /// The elements are shifted to preserve the invariants of the binary heap. | |
| /// | |
| /// The elements are shifted to preserve the invariants of the binary heap. |
| mem::swap(&mut item, self.data.as_mut_slice().get_unchecked_mut(0)); | ||
| self.sift_down_to_bottom(0); | ||
| /// Retains only the elements specified by the predicate. | ||
| /// The elements are visited in arbitrary order. |
There was a problem hiding this comment.
| /// The elements are visited in arbitrary order. | |
| /// | |
| /// The elements are visited in arbitrary order. |
2 participants
Implement #666
This PR adds the
BinaryHeap::retainmethod.I didn't base my work on #345 because the implementation seems wrong to me.
I added an internal method
BinaryHeap::removethat is now used by bothBinaryHeap::retainandBinaryHeap::pop_unchecked.In the process I improved its implementation.
One alternative implementation could be to traverse the internal vector in reverse order. This could reduce the number of shifted elements if
retainremoves multiple elements. However, this would make the implementation much more complex.