Skip to content

[13.x] Use native clamp function#59995

Closed
jnoordsij wants to merge 4 commits intolaravel:13.xfrom
jnoordsij:native-clamp
Closed

[13.x] Use native clamp function#59995
jnoordsij wants to merge 4 commits intolaravel:13.xfrom
jnoordsij:native-clamp

Conversation

@jnoordsij
Copy link
Copy Markdown
Contributor

This replaces the manual Number::clamp implementation with the PHP native clamp, which is new in PHP 8.6 (unreleased). See also:

In doing so, it also introduces some tests for 'incorrect' behavior on providing reversed bounds, which currently leads to the incorrect and potentially unexpected outcome of always returning the 'max' value. By adding a deprecation now, this functionality can later be dropped to throw the ValueError and match the new default behavior.

Comment thread src/Illuminate/Support/Number.php
@taylorotwell
Copy link
Copy Markdown
Member

If it's working fine let's just leave it.

@jnoordsij
Copy link
Copy Markdown
Contributor Author

I mainly did this again given it doesn't really work fine, as the min > max behavior is somewhat incorrect and deviates from what you might expect, hence moving towards a more universal solution would be preferable in my eyes.

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 this pull request may close these issues.

3 participants