Skip to content

Add Option::merge under option_merge feature gate #84695

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

Closed
wants to merge 1 commit into from

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Apr 29, 2021

tl;dr: add the following method to Option<T> under option_merge feature gate:

pub fn merge<F>(self, other: Option<T>, f: F) -> Option<T>
where
    F: FnOnce(T, T) -> T;

The method returns

  • Some(f(l, r)) if both options are Some(_)
  • Some(x) if either of the options is Some(x) and the other is None
  • None if both options are None

It is very similar to Option::zip_with (or Option::zip + Option::map) with the difference that it doesn't return None if only one of the inputs is None

This can be very handy when, for example, calculating a minimum of two options (without inflicting with None).


Some usage examples:

let x = Some(2);
let y = Some(4);

assert_eq!(x.merge(y, Add::add), Some(6));
assert_eq!(x.merge(y, min), Some(2));

assert_eq!(x.merge(None, Add::add), x);
assert_eq!(None.merge(y, min), y);

assert_eq!(None.merge(None, i32::add), None);

I'm not sure about the name, some other ideas:

This patch adds unstable `Option::merge` method which takes two `Option<T>`s
and a `T -> T -> T` function. If only one of the options is `Some(_)` the
method returns it. If both options are `None` the method returns `None`.
Otherwise, if both options are `Some(_)` the method merges them by applying
the given function.
@rust-highfive
Copy link
Contributor

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 29, 2021

coalesce

coalesce has a different meaning in SQL, I don't think that's a great choice:

The COALESCE() function returns the first non-null value in a list.

@jyn514 jyn514 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 29, 2021
@joshtriplett
Copy link
Member

You can write this today by treating both options as iterators and using reduce, which returns an Option that's None if the iterator is empty. Or in the specific case of min or max, you could call .min() or .max() on the iterator, which also conveniently returns an Option.

fn f(x: Option<u32>, y: Option<u32>) -> Option<u32> {
    x.into_iter().chain(y.into_iter()).min()
}

fn main() {
    assert_eq!(f(None, None), None);
    assert_eq!(f(Some(10), None), Some(10));
    assert_eq!(f(None, Some(5)), Some(5));
    assert_eq!(f(Some(10), Some(5)), Some(5));
}

However, that's certainly not the most convenient approach.

Another option, which is reasonably clear: x.xor(y).or_else(|| x.min(y))

If we consider adding this function, I'd suggest calling it reduce, as it has exactly the semantic of reduce.

@optozorax
Copy link

Some potential example usage of this method can be found here: https://doc.rust-lang.org/src/core/iter/adapters/zip.rs.html#162

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2021
@bstrie bstrie added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 2, 2021
@Dylan-DPC-zz
Copy link

r? @m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned yaahc Jun 4, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jun 5, 2021

Thanks for your PR.

I'm not convinced we should add this, though. I can't think of a lot of use cases that aren't already covered by Iterator::reduce, and I don't think it'd be clear to a reader what this function does, which I think is especially important for methods on commonly used types like Option.

Pinging @rust-lang/libs in case anyone feels like this would be an important addition. Otherwise I'll close this PR.

@joshtriplett
Copy link
Member

I've wanted this function a few times, but I don't know if it's sufficiently common to justify a function in std.

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2021
@JohnTitor
Copy link
Member

Triage: There's no comment to support this strongly since @m-ou-se left a comment, so I'm going to close. Thanks!

@JohnTitor JohnTitor closed this Jun 28, 2021
@WaffleLapkin WaffleLapkin deleted the option_merge branch June 28, 2021 12:42
@yaahc yaahc mentioned this pull request Jul 12, 2021
@WaffleLapkin
Copy link
Member Author

I've made a micro crate that provides the function, in case anyone needs it too: opt_reduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.