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

Make option_map_unwrap_or and option_map_unwrap_or_else allow by default #1192

Closed
killercup opened this issue Aug 25, 2016 · 2 comments · Fixed by #1256
Closed

Make option_map_unwrap_or and option_map_unwrap_or_else allow by default #1192

killercup opened this issue Aug 25, 2016 · 2 comments · Fixed by #1256

Comments

@killercup
Copy link
Member

One of the lints I ignore most often is option_map_unwrap_or.

The docs say:

What it does: Checks for usage of _.map(_).unwrap_or(_).

Why is this bad: Readability, this can be written more concisely as _.map_or(_, _).

I think having two method calls with one parameter makes the intent clearer than using one with two parameters (one method call per line does one thing). This may also keep the diff-noise down a bit.

The same goes for option_map_unwrap_or_else.

@leoyvens
Copy link

I've seen crates that disliked map_or so I'm in favor of this.

@Stebalien
Copy link

Worse, the argument order is confusing; personally, I'd lint against using this function. When I see map_or_else, I expect the first function to be the map function and the second to be the "else" function but it's actually backwards.

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

Successfully merging a pull request may close this issue.

3 participants