This is a tracking issue for Option::filter
.
Unresolved questions:
filter
vs and_if
vs …)@dtolnay Could you add a task "decide on the name (filter()
vs. and_then()
vs. ???)" to your comment? Also: the PR is ready now.
@LukasKalbertodt @dtolnay Updated.
I think and_if
is a brilliant name. Really great idea and strong contender. At this point I would still lean toward filter
.
I find that the and
/or
methods on Option make perfect sense and the names reflect what the method does, but it still takes me multiple seconds when I see something like or_else
or map_or
to reverse engineer what that means in terms of argument types and return types and pattern matching and behavior of my code.
In contrast, filter
is strongly immediately associated with taking a filter function &T -> bool
and using it to decide whether to keep the values in self. A short name with that sort of immediate strong association was not available for the things the and
/or
family of methods do, or they would have been called something different.
As I mentioned in https://github.com/rust-lang/rust/pull/45863#issuecomment-344633914, I have a question regarding the signature of this method:
Wouldn't this method be more flexible if the FnOnce
took &mut T
?
My suggested change would differ from Vec::retain
, which this method acts similarly to (except not in-place). Perhaps it would be worth considering changing the signature of Vec::retain
too?
I understand that this method is essentially shorthand for option.iter().filter(predicate).next()
. My suggested change would be equivalent to option.iter_mut().filter(predicate).next()
.
+1 for filter()
- this also has precedents in other languages:
Optional.filter()
Option.filter()
mfilter
(a more generic function defined for the MonadPlus
type class, but the name is still filter
-based)@nvzqz Do you have a specific use case in mind? Yes, the method would be more flexible with FnOnce(&mut T) -> bool
as predicate, but we don't necessarily want this flexibility. Simple example: Rust variables are immutable by default in order to limit what you can do -- more flexibility/possibilities are not always a good thing.
So without good use case in mind, I would keep the signature as is: I don't think anyone would expect a predicate of a method called filter()
to change the inner value.
About the name: please note that the RFC thread already contains quite a few arguments for and against certain names. I'll try to summarize the most important points here:
The two competing names are filter
and and_if
. One comment on their differences:
In other words,
.and_if
describes how it does X,.filter
describes what X is. This is the distinction between imperative and declarative programming where the.filter
is declarative.
Comparing the two:
Advantages | Disadvantages | |
---|---|---|
filter |
|
|
and_if |
|
|
How this method is called in other languages:
It looks like we're still in the bikeshedding phase here, so just leaving a note to say I found my way here after typing .filter
automatically and discovering the method exists and is unstable :)
@LukasKalbertodt FnOnce(&mut T) -> bool
would be useful when dealing with data structures that cache by mutating themselves on otherwise normal accessors. (Although you could cover this in some cases with a Cell
.) Could this be implemented in a separate Option::filter_mut
?
@bb010g Good point. However, I think I still tend to keep the closure as it is right now. A few reasons:
Iterator::filter
also takes immutable references. I guess your argument would also apply to that function, but for for the sake of consistency I'd use the same signature for Option::filter
.filter
on an Option<DataStructure>
directly: if the closure returns false
, we throw away the entire data structure.However, I'm not quite sure and don't have a very strong opinion on this decision.
You'd probably want filter_mut
for consistency with Iterator
. You can't always use Cell
due to the Copy
constraint. As for dropping the structure, this may be desirable when working with types like Arc
a lot in a concurrent environment.
So I guess we keep this issue just for filter
. Adding filter_mut
or something similar can then be proposed via another RFC, I'd say.
Then there's only the naming issue left. Here again, I don't have a strong opinion. I like both names a lot and would be happy either way.
There are not very many and_*
functions. It's only and
and and_then
and one of those was a poor choice. and
is fitting in the logical sense of the word. and_then
loses that meaning and is purely temporal. It could have just as well been used for map
. The proper name for and_then
is flat_map
.
The and_if
name evokes no meaning in my head. And if X then what? The name works only with convention but that convention doesn't exist. filter
is well established already. We would need good reasons to deviate from this.
There are a bunch of *_or*
variants of other functions and for all of them it's immediately obvious what they do and what kind of data they expect from the operation they extend.
the RFC thread already contains quite a few arguments for and against certain names
As it seems unlikely that more time will produce new arguments either way, I’d like to propose stabilizing this feature as it currently is in Nightly.
@rfcbot fcp merge
Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed.
Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
:bell: This is now entering its final comment period, as per the review above. :bell:
The final comment period is now complete.
Most helpful comment
It looks like we're still in the bikeshedding phase here, so just leaving a note to say I found my way here after typing
.filter
automatically and discovering the method exists and is unstable :)