Skip to content

Method for bool to Result<(),E> conversion #3777

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
RobWalt opened this issue Feb 22, 2025 · 5 comments
Closed

Method for bool to Result<(),E> conversion #3777

RobWalt opened this issue Feb 22, 2025 · 5 comments

Comments

@RobWalt
Copy link

RobWalt commented Feb 22, 2025

Prior work

#2606 and #2757 was a really useful addition for writing combinator-style rust code. Thanks a lot for that!

The Motivation

After using the new methods for quiet some time, there have been situations here and there that still feel a little wonky, especially dealing with bool to Result situations.

Currently, we have the following options:

  • plain old ifs
    if condition {
      Ok(ok)
    } else {
      Err(err)
    }
  • detour over then/then_some
    condition.then_some(ok).ok_or(err)

While the latter works fine for most of the situations, I kind of always refrain from using it if I just want to get a Result<(), E> since it looks unnecessarily verbose and arcane:

condition.then_some(()).ok_or(err)

It's not only verbose but also reads very poorly in real world scenarios like early exit conditions and run condition checks:

fn may_fail() -> Result<(),E> {
  is_precondition_met.then_some(()).ok_or(err)?;
  // ...
  Ok(())
}

The Proposal

I think it could be nice to support some method like or_err as follows:

impl bool {

  fn or_err<E: Error>(self, err: E) -> Result<(), E> {
    if self {
      Ok(())
    } else {
      Err(err)
    }
  }

}

So effectively just handling the Result<T, E> where T = (). This would "improve" the situation above to become

fn may_fail() -> Result<(),E> {
  is_precondition_met.or_err(err)?;
  // ...
  Ok(())
}

This would also mean we call only one method (or_err) instead of two methods (then_some, ok_or). I guess this has negligible impact though since the compiler most likely optimizes this to the same code (?) (not experimentally tested!).

Last but not least, there would probably also be the need for a closure-taking variant or_else_err or something alike.

Fair arguments against the change

  • increases the std API surface further
    • I'm not sure about this one. I also feel like Idea: Built-in way to go from bool to Option #2606 opened the gate for this in general, so the baseline discussion about custom methods on bool doesn't have to take place again. The only real question would be: How far are we going with things like this?
  • there are more ways to do the same thing. If T != (), then the following would be equivalent
    condition.then_some(ok).ok_or(err);
    condition.or_err(err).map(|_| ok);
    condition.or_err(err).and(Ok(ok));
    // probably more
  • I would assume that there are situations where it's not desirable to use code like this over plain old ifs, although I can't mention any off the top of my head. In the end some performance considerations could be added to the docs or clippy Lint's could be established for that
  • the change doesn't really do enough for the efforts needed to implement them

Feedback

Maybe this is just me being weird with my rust in the end. In that case, feel free to close this issue/RFC if there's no clear indication other people feel a need for this feature. Thanks for giving it a read anyways!

@traviscross
Copy link
Contributor

You're unlikely to get a lot of feedback here. If you just want to discuss it generally, you'd be better off opening a thread on Zulip or IRLO. If you want to make a proposal, you'd create a new issue and select "API Change Proposal" (i.e. ACP) here:

https://github.com/rust-lang/libs-team/issues/

@traviscross traviscross closed this as not planned Won't fix, can't repro, duplicate, stale Feb 22, 2025
@RobWalt
Copy link
Author

RobWalt commented Feb 22, 2025

Thanks a lot for the guidance and sorry for the unnecessary noise in the wrong place!

@shepmaster
Copy link
Member

For prior art, see macros like https://docs.rs/snafu/latest/snafu/macro.ensure.html

@nielsle
Copy link

nielsle commented Feb 22, 2025

For the record. The following crate has syntactic sugar.

https://crates.io/crates/then

use then::Some;

assert_eq!(false.some(0), None);
assert_eq!(true.some_with(Default::default), Some(0));

@moxian
Copy link

moxian commented Feb 24, 2025

https://docs.rs/boolinator/latest/boolinator/trait.Boolinator.html is another crate serving the same purpose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants