Skip to content

Suggest usage of Ord::clamp, f32::clamp, and f64::clamp to simplify code #9477

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
Xaeroxe opened this issue Sep 14, 2022 · 2 comments · Fixed by #9484
Closed

Suggest usage of Ord::clamp, f32::clamp, and f64::clamp to simplify code #9477

Xaeroxe opened this issue Sep 14, 2022 · 2 comments · Fixed by #9484
Labels
A-lint Area: New lints

Comments

@Xaeroxe
Copy link
Contributor

Xaeroxe commented Sep 14, 2022

What it does

Identifies good opportunities for a clamp function from std or core, and suggests using it.

clamp functions

f32::clamp
f64::clamp
Ord::clamp

Lint Name

clamping_without_clamp

Category

complexity

Advantage

It's much shorter and easier to read, also doesn't use any control flow.

Drawbacks

The clamp functions panic in some circumstances which the original patterns will merely malfunction on. Namely, Ord types will panic if max < min and the floating point functions will additionally panic if min.is_nan() or max.is_nan(). Some may consider this a perk, but I'm listing it as a drawback because it may introduce panics where there weren't any before.

Example

if input > max {
    max
}
else if input < min {
    min
} else {
    input
}
input.max(min).min(max)
match input {
    input if input > max => max,
    input if input < min => min,
    input => input,
}

Could all be written as:

input.clamp(min, max)
@Xaeroxe Xaeroxe added the A-lint Area: New lints label Sep 14, 2022
@Xaeroxe Xaeroxe changed the title Suggest usage of Ord::clamp, f32::clamp and f64::clamp to simplify code Suggest usage of Ord::clamp, f32::clamp, and f64::clamp to simplify code Sep 14, 2022
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Sep 14, 2022

Another clamp anti-pattern

let mut x = input;
if x < min { x = min; }
if x > max { x = max; }

Should be

let x = input.clamp(min, max);

@kylecarow
Copy link

kylecarow commented Jun 7, 2024

Another anti-patten exists in chained min/max calls:

input.min(max).max(min)

and

input.max(min).min(max)

Should both be

input.clamp(min, max)

EDIT: ah, looks like this has been mentioned before: #5854 #6751

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants