Rust: Tracking issue for Result::map_or_else

Created on 11 Aug 2018  Â·  16Comments  Â·  Source: rust-lang/rust

Result should also have map_or_else(), like Option (but the closure for the else case should also get the Err as argument, like with unwrap_or_else).

Both Option and Result have unwrap_or_else and unwrap_or, so they should also both have map_or_else.

C-tracking-issue T-libs

Most helpful comment

I think its parameters should be changed
from
fn map_or_else(self, fallback: F, map: M)
to
fn map_or_else(self, map: M, fallback: F)

All 16 comments

Implemented in #53777

I think its parameters should be changed
from
fn map_or_else(self, fallback: F, map: M)
to
fn map_or_else(self, map: M, fallback: F)

It would also be good to have Result::map_or.

@liigo Yes, having the fallback as second argument makes more logical sense because it mirrors the if else structure, but this would be a breaking change now :(

@Boscop That's not an issue because it's still unstable. In most cases the error is of a different type than the value and it would cause a compile time error, anyway.

@Emerentius I agree that having the ok handler as first arg would make more sense, but I'm not sure how we can change it now.. map_or_else's arg order was chosen to be consistent with map_or. We would have to change the order for both then. I guess you'd have to do a RFC to suggest changing this.

map_or was flipped from the obvious order so that the typically short default case would be in front of the potentially long closure. Breaking consistenty with that is less troublesome than the map_or_else precedent from Option which is stable, already.
Looks like we're stuck with it…

Look on the bright side. The original ergonomic argument that the typically longer closure should be at the end is still compelling and provides a reasonable basis for making consistent decisions about this sort of thing.

To second what was mentioned above, it would make a lot of sense to add Result::map_or at the same time.

Is there any reason this shouldn't just be the tracking issue for map_or as well? To me, it seems like they can fall under the same feature flag (even if it won't be as accurately named anymore).

Seconding map_or request

66292 is adding map_or. Feedback welcomed.

I'm sorry but this implementation of map_or_else is incredibly annoying as it differs from Option.
When the original issue says:

Result should also have map_or_else(), like Option (but the closure for the else case should also >get the Err as argument, like with unwrap_or_else).

Both Option and Result have unwrap_or_else and unwrap_or, so they should also both have >map_or_else.

and the parameters are then switched, is mildly said absurd! - keep things consistent!

Oof. Indeed. Not only are they switched from Option, but they're backwards from the name too. I really hope it's not too late. Does it need a new feature gate name or is that considered "nightly problems" too?

Stabilization PR in #66322

Looks like this is released with 1.41 but the order flip is really quite a pain.

Any precedent for why error first?

@cdbattags Seems described in the first comment https://github.com/rust-lang/rust/issues/53268#issue-349735810.
I think it is "ok second" rather than "error first", similar to Option::map_or_else.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Robbepop picture Robbepop  Â·  3Comments

dnsl48 picture dnsl48  Â·  3Comments

behnam picture behnam  Â·  3Comments

modsec picture modsec  Â·  3Comments

zhendongsu picture zhendongsu  Â·  3Comments