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

Use Vec::drain_filter when available #522

Closed
martinthomson opened this issue Apr 6, 2020 · 3 comments
Closed

Use Vec::drain_filter when available #522

martinthomson opened this issue Apr 6, 2020 · 3 comments
Labels
p2 Issues that we want to fix

Comments

@martinthomson
Copy link
Member

martinthomson commented Apr 6, 2020

This code has bothered me for a while, because it seems inefficient (two inverse filters means redundancy). It's better than dumb iteration, but it isn't perfect.

let r = self
.blocked_streams
.iter()
.filter(|(_, req)| *req <= base)
.map(|(id, _)| *id)
.collect::<Vec<_>>();
self.blocked_streams.retain(|(_, req)| *req > base);
Ok(r)

Turns out that Vec::drain_filter is coming. When that becomes stable, we should move to using that. drain_filter(|i| {/* logic here */}).collect() will be much nicer.

@MightyPork
Copy link

MightyPork commented May 15, 2020

ftr, I solve a similar problem with the combination of drain.filter_map.collect, moving the removed elements to a separate vec (or consuming in other ways) in the filter_map callback

rust-lang/rust-clippy#4433 (comment)

@ddragana ddragana added the p2 Issues that we want to fix label Nov 24, 2021
@larseggert
Copy link
Collaborator

@martinthomson this is now

let r = self
.blocked_streams
.iter()
.filter_map(|(id, req)| if *req <= base_new { Some(*id) } else { None })
.collect::<Vec<_>>();
self.blocked_streams.retain(|(_, req)| *req > base_new);

Anything left to do?

@martinthomson
Copy link
Member Author

This would be better with drain_filter(), but what we have is fine. Given that drain_filter is taking SOOOO long to stablize, we might just give up on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2 Issues that we want to fix
Projects
Status: Closed
Development

No branches or pull requests

4 participants