rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

RFC: map_or_default in Option and Result

Open orlp opened this issue 4 years ago • 16 comments

This adds Option::map_or_default and Result::map_or_default which behave exactly like .map_or_else(Default::default, _) would, similarly to .unwrap_or_default().

Rendered.

orlp avatar Jul 14 '21 13:07 orlp

Echoing a comment from the internals thread: once we have the free function default, we could write this as map_or_else(default, ...).

I think we should weigh that when deciding if we want to add this. It may still make sense to add this.

joshtriplett avatar Jul 14 '21 15:07 joshtriplett

As was also started in the internals thread, I am of the opinion this doesn't need an RFC.

jhpratt avatar Jul 14 '21 15:07 jhpratt

As was also started in the internals thread, I am of the opinion this doesn't need an RFC.

It's said here to submit an RFC if it is a new API. Maybe it could be argued it's a "Obvious API hole patch", but I'm not so sure. The RFC was easy enough to write, if the libs team thinks it does not need an RFC no harm done and I can make a pull request.

orlp avatar Jul 14 '21 15:07 orlp

Note that resize_default was deemed unnecessary in https://github.com/rust-lang/rust/issues/41758#issuecomment-449719961 -- I think the same "just pass the function" may well apply here.

scottmcm avatar Jul 14 '21 17:07 scottmcm

Note that resize_default was deemed unnecessary in rust-lang/rust#41758 (comment) -- I think the same "just pass the function" may well apply here.

I've got a feeling this would be used far more often than resize_with_default.

jhpratt avatar Jul 14 '21 17:07 jhpratt

I don't know how much of an argument this is, but in my opinion the methods map_or and map_or_else are terribly named anyways, because they don't actually constitute a “mapping” operation. They are just a method form of a general by-value match statement. The only relation to “map” comes from the fact that you can write them as a chain of calls .map(…).unwrap_or[_else](…).

So my argument would be: if map_or[_else] are badly named, we don't need more methods with similarly bad names.

Im not having a great suggestion for alternative names right now, and I'm not sure if there was previous discussion about this.

If a new alternative name for map_or_else was shorter (maybe by dropping the pattern of making the version taking a closure the longer name, similar how bool::then did although the proposed then_some has a weird name, too, because I don't know how that name translates to “like then but doesn't take a closure”) then the or_default alternative would be needed even less. As things stand now, it seems like the main advantage of map_or_default would be in dnot needing to write or_else. E. g. if a new pair of names was .cases (for map_or_else) and .cases_with_value (for map_or) then an alternative .cases_with_default for .cases(default, …) would be highly redundant since the alternative using default would even be shorter. Consider these names mostly place-holders.

steffahn avatar Jul 15 '21 07:07 steffahn

Shall we deprecate map_or / map_or_else?

arniu avatar Jul 28 '22 15:07 arniu

No.

BurntSushi avatar Jul 28 '22 15:07 BurntSushi

Then why not add map_or_default?

arniu avatar Jul 28 '22 15:07 arniu