Skip to content
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

Documentation on Result::map_or_else uses very unfortunate wording #88195

Closed
orlp opened this issue Aug 20, 2021 · 5 comments · Fixed by #88772
Closed

Documentation on Result::map_or_else uses very unfortunate wording #88195

orlp opened this issue Aug 20, 2021 · 5 comments · Fixed by #88772
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@orlp
Copy link
Contributor

orlp commented Aug 20, 2021

The type signature of Result::map_or_else:

pub fn map_or_else<U, D, F>(self, default: D, f: F) -> U where
    F: FnOnce(T) -> U,
    D: FnOnce(E) -> U, 

Note how default is the function that gets applied to the Err variant. Then, the docstring (emphasis mine):

Maps a Result<T, E> to U by applying a fallback function to a contained Err value, or a default function to a contained Ok value.

I think this is a very unfortunate choice of words and should be changed.

@jyn514
Copy link
Member

jyn514 commented Aug 21, 2021

@orlp are you interested in making a PR to fix it? :) there are instructions in https://rustc-dev-guide.rust-lang.org/getting-started.html#building-and-testing-stdcorealloctestproc_macroetc

@jyn514 jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 21, 2021
@orlp
Copy link
Contributor Author

orlp commented Aug 21, 2021

I could, but I think I'd prefer to bikeshed a bit first as to how it should be described.

@MightyPork
Copy link

To me these functions are so confusing that I practically never use them, the arguments are in the wrong order ... as repeatedly noted in both the tracking issue and stabilization PR that added it. It can't be changed, of course :/

I rather like the way it's documented in the corresponding method on Option: https://doc.rust-lang.org/std/option/enum.Option.html#method.map_or_else

@mdsn
Copy link
Contributor

mdsn commented Aug 27, 2021

How about

Maps a Result<T, E> to U by applying function f to a contained Ok value, or by applying default to a contained Err value.

It explains in the same order as the function name (ok_or_else) and doesn't overload the meaning of "default" in the context of the unfortunate function argument.

@orlp
Copy link
Contributor Author

orlp commented Sep 9, 2021

Made a pull request using @mdsn 's suggestion, only replacing "applying default" with "applying function default" to be consistent with the earlier part of the sentence.

Manishearth added a commit to Manishearth/rust that referenced this issue Oct 7, 2021
…aahc

Fixed confusing wording on Result::map_or_else.

Fixes rust-lang#88195.
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 8, 2021
…aahc

Fixed confusing wording on Result::map_or_else.

Fixes rust-lang#88195.
@bors bors closed this as completed in 2b6d7f7 Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants