[Pre-RFC] Option::for_some

libs
1
  • Feature Name: option_for_some
  • Start Date: 2020-08-28
  • RFC PR:
  • Rust Issue:

Summary

This RFC proposes the addition of Option::for_some.

Motivation

Option lacks an operation that consumes the value in a similar way as Iterator::for_each. It is possible to use if let:

fn main() {
    let number: Option<u32> = Some(42);

    if let Some(even) = number.filter(|n| n % 2 == 0) {
        println!("{} is even", even);
    }
}

However, this expression can be quite mouthful and does feel good when chaining multiple operations. Where a similar chain with an Iterator could simply resort to for_each, Option offers nothing. With Option::for_some the example above could be more compact:

fn main() {
    let number: Option<u32> = Some(42);

    number
        .filter(|n| n % 2 == 0)
        .for_some(|even| println!("{} is even", even));
}

A trait-based implementation of the for_some function:

pub trait OptionForSome<T> {
    fn for_some<F: FnOnce(T)>(self, f: F);
}

impl<T> OptionForSome<T> for Option<T> {
    fn for_some<F: FnOnce(T)>(self, f: F) {
        if let Some(value) = self {
            f(value);
        }
    }
}

Rationale and alternatives

This would provide a compact alternative to if let construction to consume Some(value). The alternative could be more ergonomic and natural in operation chains. It would provide a match for Iterator::for_each as well.

1 Like
2

As far as I can tell, this is the Option::map function, minus the return value.

fn main() {
    Some(42).filter(|x| x % 2 == 0).map(|x| println!("{} is even", x));
}
11 Likes
3

You are right, but there are some points why map IMO does not feel right:

  • This might be just subjective, but I'd expect map to map the values, while for_some is more focused on side-effects. It is a "terminal" operation.
  • The example with map instead causes Clippy to be unhappy.
  • With the respect to Clippy, I'd argue that for_some indicates better the intent of consuming the value.
2 Likes
4

I'm not sure I'd agree with that clippy lint.

In any case, I'm not expressing strong opposition to the proposed method (though I'd probably nitpick the name a little). I'm more suggesting that if we were to have it, it'd likely be implemented as "map with a different name and without the return value", and we should evaluate it accordingly.

On a different note, I'd also like to have a try_map function, which would allow using a function that can fail with an error, and would return Result<Option<_>, _> rather than Option<Result<_, _>>, so that .try_map(...)? would work.

4 Likes
5

A way to make clippy happy is x.map_or((), |y| ...).

1 Like
6

On a different note, I'd also like to have a try_map function, which would allow using a function that can fail with an error, and would return Result<Option<_>, _> rather than Option<Result<_, _>> , so that .try_map(...)? would work.

I'd probably need a more complete example to see where transpose would not work for you.

3 Likes
7

you can .map(...).transpose()

Edit: @pdolezal was faster

4 Likes
8

It would work, it's just less convenient, less self-explanatory, and more opaque.

Much the same reason you're asking for for_some when we already have map.

Which looks clearer?

let y = some_opt.try_map(|x| {
    foo(x)?;
    Ok(bar(x)?)
})?;

vs

let y = some_opt.map(|x| {
    foo(x)?;
    Ok(bar(x)?)
})
.transpose()?;
2 Likes
9

Moreover, if one does agree with the clippy lint, then the exact same readability concern would apply to all possible usages of for_some().

I've used Option::map like this and it seems suitable. Iterators need a separate for_each method because Iterator::map is lazy, but Option::map isn't, and the name "for_some" isn't the most immediately recognizable as to its meaning. If you need to be explicit, there's always option.into_iter().for_each(), which is very clear about its intent and only a smidgen more verbose.

6 Likes
10

I'll try to summarize the replies and arguments raised so far with some comments. Feel free to comment the summary and bring more thoughts :slight_smile:

Option::map does the same job.

Both map and for_some invoke an operation that consumes the contained value. However, map produces another Option with the operation result. As the name suggests, map is supposed to map values, so that the outcome could be used in chained calls. On the contrary, for_each is supposed to consume the value and produces nothing useful (as far as we could consider () here as nothing).

Clippy warning seems confusing.

If map should be used for consuming the value, statements like option.map(|value| foo(value)); should not issue any warning.

If for_some existed, the warning still makes sense, although it could be refined in the sense that the result of map is not used and the intent is not clear (for instance: does it mean that someone forgot to use it or does it mean that map has some side effect?) and there are clearer alternatives like if let Some(value) and for_some.

Use option.into_iter().for_each() instead.

A hard argument against this idea is that Iterator::for_each takes FnMut, while for_some takes FnOnce as it can take the advantage of at-most-once invocation.

A soft argument is that converting Option into Iterator just to consume the at most one value seems too convoluted and instead of indicating the intent clearly and expressing it in a compact way, it brings more complexity. I'd guess that a newbie could think: Why the heck an iterator appears here?

Name is confusing.

Naming is always somewhat subjective, but as long as most users agree that the name fits the intent and feels good in the code, I guess the name is fine. I would trust English native speakers more with the name choice than I trust myself. Anyway, let me share some of my (subjective) reasons for choosing for_some.

I like Option::for_some mostly because it's similarity with Iterator::for_each (and it seems to me natural as well). I considered a few other names, the most serious alternative was if_some, which nicely resembles the if let Some(value) pattern, but nothing in std starts if_, so it feels less familiar. Maybe yet, if_some feels like there should be if_none as well (I don't have the feeling with for_some, since for_none looks weird) and this way leads to ugly ends (basically poking users to avoid match for Option even when it is clean).

I'm not in favor in using map as a part of the proposed operation's name. I guess it is obvious from the first point's summary: I feel map should map a value, while this operation is supposed to consume a value. I considered even consume as the name. Maybe some people would like it more.

Just a final footnote about try_map that @josh mentioned as something similar. I think his idea is closer to the case of filter_map, which allows using filter and map used in conjunction in a more user friendly way. Then try_map could be understood as a similar operation with the respect to map and transpose (if I interpret it correctly). Anyway, I see no strong links to proposing for_some and I don't think the proposals must be necessarily judged together.

3 Likes
11

You didn't pick up my suggestion of using .map_or((), ...).

12

A question: what would be the Result equivalent(s) of this? Usually they parallel each other fairly closely in the methods on offer (as in many ways Option<T> <=> Result<T, ()>) so whether for_ok/for_err would be good/bad things might be indirect evidence for/against this method.

Note that .for_each() on iterators didn't exist for a long time, with the advice being to just use a for loop: Add for_each function to Iterator trait by aochagavia · Pull Request #14911 · rust-lang/rust · GitHub

That only changed thanks to two things: 1) rayon's ParallelIterator has it, so it's helpful when switching between that and Iterator 2) there are some iterators where internal iteration is more efficient. Notably the "better for chaining" argument never tipped the scales.

Option::map has value in that it avoids needing to specify else { None }, but else { () } is implicit so that doesn't apply to Option::for_some. And there's no rayon or internal iteration argument for this method.

5 Likes
13

Ah, sorry :flushed: I guess I could make a poll. Perhaps some more suggestions will yet arrive.

Edit:

I missed your point, probably was already too tired :roll_eyes: I agree that option.map_or((), |value| use(value)) is a quite nice alternative. Another way to make Clippy happy is to assign & forget: let _ = option.map(|value| use(value)).

The assign & forget still seems to me not very nice. I admit that map_or looks better, but I'm still not convinced that it voids the value of for_some completely. But I will use it until for_some is available :wink:

14

Good remarks and questions. I didn't know the history – thanks. I will have to think your questions over.

Regarding the "better for chaining" argument: it might not be raised then, especially when it was (maybe?) not so well established, but it might be valid and it might be valid even more now.

15

@scottmcm I have some answer, maybe not very satisfying, but I didn't want to leave your question unanswered.

A question: what would be the Result equivalent(s) of this? Usually they parallel each other fairly closely in the methods on offer (as in many ways Option<T> <=> Result<T, ()>) so whether for_ok/for_err would be good/bad things might be indirect evidence for/against this method.

I'm not entirely sure if something like for_ok would have a good meaning or use. I agree with you that similarity between these types should be considered, so I guess I should bring some arguments why Option::for_some makes sense, while Result::for_ok probably not. Although there are similarities, there are differences as well (why to have two types otherwise, right?). This might be one of the cases where the differences show up.

The primary meaning of Result is representing success or failure. Uses of Result are focused on function results and processing them in a safe way. Ignoring Result smells, the cases where neither Ok nor Err need any processing are probably very rare. Err cases can be thrown away easily with ? operator, focusing the code flow on the happy path and the Ok value.

How the hypothetic Result::for_ok could be helpful and how it would fit in? I would expect it to help consuming the Ok value and I would expect both these options to work as noted:

// Handle Err as usual
fallible_operation().for_ok(|ok| println!("We successfully generated {}.", ok))?;

// Ignore Err completely
fallible_operation().for_ok(|ok| println!("We successfully generated {}.", ok));

In both cases the value of the statement would be (). Compare it to the code without for_ok:

// Handling Err
let ok = fallible_operation()?;
println!("We successfully generated {}.", ok);
// Alternatively
println!("We successfully generated {}.", fallible_operation()?);

// Ignoring Err
if let Ok(ok) = fallible_operation() {
    println!("We successfully generated {}.", ok);
}

When handling Err, for_ok does not help much in my opinion. However, with for_ok it is easy to ignore Err, even by an accident. It seems dangerous to me and I think this looks like KO for for_ok.

Regarding for_err, I think it is a lost case from the beginning and not just because for_ok seems to lead in a bad direction. As I noted before, I don't see something like Option::for_none as a good idea. The less reasons there I would expect in favor of Result::for_err.

Why the failure with Result::for_ok might not be relevant for rejecting Option::for_some? Option is more generic and it does not (or should not) indicate success/failure. Therefore not acting on None (or Option in general) does not smell like ignoring Err (or Result in general). So, a short answer might be: because for_some brings no danger.

By the way, for_some could indirectly extend the possibilities of Result:

// Ignore Err completely, but in a safer way
fallible_operation()
    .ok()
    .for_some(|ok| println!("We successfully generated {}.", ok));
1 Like
16

I don't know if there's an explicit statement about this value somewhere, but I think in general, Rust tends to favor "brace" statements for side effects over eager, chained method calls. One example would be that this:

for x in foos().map(bar) {
    baz(x);
}

is typically preferred over:

foos().map(bar).for_each(baz);

Maybe that's just my personal preference shining through, but I thought I read about that somewhere in the Rust book or something like that :man_shrugging: I personally think it helps emphasize the fact that we're depending on side effects rather than traditional "functional-esque" (no side-effect) programming.

This would be analogous to how this:

if let Some(x) = foo().bar() {
    baz(x);
}

would be preferable over this:

foo().bar().for_some(baz);

I can empathize that it would be nice if there were an analogous method, though, for nothing other than consistency :smile:

4 Likes
17

This honestly has precedent in java, with java.util.Optional::ifPresent, which accepts a Consumer<T> (which is effectively, at least here, an FnOnce(T)->()). Similar to here, it can be done using the stream api (comparable to rust Iterators). I'd support it at least on the prior art side

1 Like
18

Yes, you are right. I didn't mention that deliberately, but it might be a good point after all.

I'll try to explain in detail for those less familiar with Java. Optional::ifPresent is basically a must, just compare:

// With ifPresent
thatOptional.ifPresent(System.out::println);

// Without ifPresent
if (thatOptional.isPresent()) {
    System.out.println(thatOptional.get());
}

Optional::get is like Option::unwrap, throwing an exception when there is nothing to unwrap. Hence just seeing it makes one more suspicious.

Optional::map can't work here, because map must return something, but System.out::println has void return type, which means it returns literally nothing. Yes, not having a unit type is a tragedy. On the other hand, it is obvious that the operation in ifPresent has side effects (because it can't return anything), while map with its Function argument type is generally expected to rather avoid side effects (it is not forbidden, but it is not considered nice either).

It was mentioned the possibility of turning an Option into an iterator and using Iterator::for_each. This is possible with Optional too (since Java 9):

thatOptional.stream().forEach(System.out::println);

Still, Optional::stream is never used like this and it exists mostly for using with Stream::flatMap to extract values from a Stream<Optional> (I guess everybody can imagine it).

The case of Java Optional shows a few reasons that are not applicable for Rust (like mitigating its awkwardness with the respect to the language's other features) and these reasons led me rather not using it as an argument. Instead, I tried to pick the reasons that I considered valid for Rust as well:

  • A more compact invocation of operations that consume Option's value.
  • A distinct operation that is supposed to have side effects (unlike map and friends).
  • An analogy to Iterator::for_each, but a better alternative to the proposal of using option.iter().for_each(…), which would be more compact and which would allow FnOnce instead of FnMut.

Anyway, I'm biased: Java is my bread and buffer and therefore the chained-method-invocation style with a terminal operation like ifPresent might feel more natural for me than for other developers using Rust. I tried, meanwhile, using Option::map_or instead, as I was suggested, and although it works, kind of, I still feel not comfortable enough. The distinct semantics of ifPresent/for_some is missing. So I hope that Option::for_some still may bring some value.

4 Likes
19

Thank you for the educational explanation. It makes sense.

20

I'd probably use apply and do my_option.map(…).apply(drop);.