rust-lang/rfcs

Add filter to Option

zorgiepoo opened this issue · 24 comments

Coming from Scala, I miss not having a filter method on Option. I believe the function signature would be:

fn filter<P: FnOnce(&T) -> bool>(self, predicate: P) -> Self

Last use case I was using it for:

let valid_line = |x: &str| -> bool { x.trim().len() > 0 && !x.starts_with("#") };
let input_line: Option<String> = ...; // read from somewhere
let line = input_line.filter(|x| valid_line(&x));

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 use and_then, or write your own trait implementation. If this feature is desired, I'd be happy to send a pull request. Using and_then is probably how I'd implement it:

self.and_then(|x| if predicate(&x) { Some(x) } else { None })

On Feb 4, 2016, at 9:18 AM, Kevin K. notifications@github.com wrote:

I think Option::map_or already does this to some extent

fn valid_line(x: &str) -> Option<&str> { x.trim().len() > 0 && !x.starts_with("#") { Some(x) } else { None } }

let input_line: Option = ...; // read from somewhere
let line = input_line.map_or(None, |x| valid_line(&x));

Reply to this email directly or view it on GitHub #1485 (comment).

That seems similar to the and_then way except it’s moving the complexity to the valid_line function which no longer returns a boolean.

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 filter would be a little more simple 😜

aji commented

👍

When writing code that uses lots of Options, I often chain my calls like this:

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 filter to insert into this would simplify an operation that I feel is underrepresented. The existing solution is wordy, and not immediately clear:

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.

kstep commented

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 filter to Option.

k3d3 commented

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 and_then) is possible now though: some_optional().into_iter().filter(predicate).next(). No idea whether code gen would be optimal.

I was hesitant to write an RFC myself, because my option-filter crate has seen so little use. As of this writing, it has zero reverse dependencies and only 662 downloads all time. An empty placeholder gets more downloads than that!

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 option-filter crate having ≈0 users isn't conclusive evidence towards a "no", but it's pretty close. So if you want this feature to land in std, the best way to show your support is to use it – by adding option-filter to your project ☺

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.

kstep commented

@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.

F001 commented

@dhardy I would like the name retain instead of filter. It is more precise IMO. And many containers in Rust already have this method.

@F001 retain filters the container in-place (&mut self), which is a different thing altogether.

F001 commented

@lfairy Oh, thanks. I overlooked the receiver is self. I think filter(self, pred) and retain(&mut self, pred) can coexist for different circumstances.

I'll just add my comment here that I would have just found filter for std::Option to be very useful, but wrote out the code fully instead of adding the option-filter crate as another dependency.

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 .filter working here? It would catch readers.