-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add filter to Option #1485
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
Comments
P.S. Your reply showed up to me via e-mail but not on GitHub’s web interface. Very odd :|. |
@zorgiepoo I had deleted it because I decided I liked the filter option better...even though it's technically possible already, using a |
👍 When writing code that uses lots of let result = something()
.and_then(|x| do_something_with_x(x + 1))
.map(|y| process_y_a_little(y + 1))
.unwrap_or(0); Having something like let result = something()
.and_then(|x| do_something_with_x(x + 1))
.and_then(|z| if some_predicate(z + 1) { Some(z) } else { None })
.map(|y| process_y_a_little(y + 1))
.unwrap_or(0); versus the proposed solution, let result = something()
.and_then(|x| do_something_with_x(x + 1))
.filter(|z| some_predicate(z + 1))
.map(|y| process_y_a_little(y + 1))
.unwrap_or(0); which I consider to be a significant improvement. |
I also from Scala land, so 👍 to this feature! I missing the method very badly. |
trait Filter<T> {
fn filter<P: FnOnce(&T) -> bool>(self, predicate: P) -> Self;
}
impl<T> Filter<T> for Option<T> {
fn filter<P: FnOnce(&T) -> bool>(self, predicate: P) -> Self {
match s {
Some(t) if predicate(&t) => Some(t),
_ => None,
}
}
} You could have this as a crate called OptionTools or something. |
Yeah, you could but I see this is as an omission from the standard library. Option has map, and flat map (and_then), so why not filter? Another usage came to my mind, where you don't already start off with an existing Option: if (x.len() > 0) { Some(x) } else { None }
//..versus..
Some(x).filter(|x| x.len() > 0) |
Some(x).into_iter().filter(|x| x.len() > 0).next() I think this would be nice, I'm just suggesting similar solutions that can be used right now. |
I've written up a teeny crate for those who want to play with this method. |
+1 on adding |
Agreed, having filter on Option types would be very helpful |
One more that misses this convenience from Scala. It makes sense for this to be a standard idiom, and hence in std, and FWIW, I am generally very in favor of a minimal std. |
Because this issue is still getting activity, I want to state that I will not have the time to propose/create a proper RFC for this, so if anyone wants to, feel free to give it a try. One of the Rust developers before told me this addition would be considered a "substantial" change (if you read the repo Read Me). I don't know what the developers think about the feature, however. The code changes to add this feature and add it to the tests is not that much. |
Edit: okay, I'll write a little RFC for this later. I'm not really from Scala land but would still like this. An alternative (besides |
I was hesitant to write an RFC myself, because my When @dhardy (or whoever else) posts their RFC, one of the first questions to be asked will most certainly be: "will this method be used?" The |
But @lfairy that's another dependency. Many people (certainly myself) will be wary of adding another dep for such a small gain. But thanks for the reminder. I lost track of this one somehow. |
@lfairy the crate you mentioned has more downloads because it's just older than yours. |
@dhardy While that wariness would make sense in another context, I don't think it's useful to trust that instinct here. Rust's tooling encourages small libraries, and the core team have in the past encouraged developing out-of-tree. And the usual arguments against (trust, bloat, dependency hell) aren't as strong with a crate of this size and scope. @kstep Good point – I should have compared the dates more carefully. I need to head off now, but in the mean time if you can find an example from a closer date then I'll be happy to replace it with that. |
@dhardy I would like the name |
@F001 |
@lfairy Oh, thanks. I overlooked the receiver is |
I'll just add my comment here that I would have just found |
This seems like a super reasonable addition to Option, and one which would've made code that I've written nicer. |
I just needed this and landed here, +1. |
re: @lfairy 's library I have not used it because that makes my code different. Why is |
Coming from Scala, I miss not having a filter method on Option. I believe the function signature would be:
Last use case I was using it for:
There may be other use cases since we live with strings and collections, and sometimes we want to not consider "close" to zero length. The idea would be to filter out "undesired" options.
Workarounds: turn the option into an iterator and call
next()
, or useand_then
, or write your own trait implementation. If this feature is desired, I'd be happy to send a pull request. Usingand_then
is probably how I'd implement it:The text was updated successfully, but these errors were encountered: