> On 24 Jun 2021, at 1:50 AM, tyson andre <tysonandre775@hotmail.com> wrote:
>
> So I think the major objections are that:
>
> 1. This is easy to implement in userland and there's negligible performance benefit
> (in code that is a performance sensitive loop, it may still be worse compared to `$num < $min ? $min : ($num > $max : $max : $num)` when validity of types and min/max are known, especially with the JIT)
> 2. People not being sure if they'd ever use it personally, especially with the ability to customize parameter order and permitted argument types in userland
> 3. It's inconsistent with min() and max(), which support any comparable type such as GMP(https://www.php.net/manual/en/class.gmp)
> (arbitrary precision numbers), DateTime, etc., and that may lead to surprises.
> Although PHP's comparison operator and https://www.php.net/manual/en/function.min.php have many, many inconsistencies, already
>
> (Though special casing GMP is probably a bad idea due to it being optional and having an incompatible license with core for packagers)
> 4. I'm not sure what this is meant to do with the float NAN (Not A Number) from the RFC description, but that's solvable
> On 24 Jun 2021, at 2:15 AM, tyson andre <tysonandre775@hotmail.com> wrote:
>
> I'd strongly prefer an actual benchmark for context and accuracy - is it actually faster or slower than the most efficient userland implementation and by how much?
> E.g. in an optimized NTS build with `CFLAGS=-O2`, opcache enabled(zend_extension=opcache, opcache.enable=1,opcache.enable_cli=1),
> and no debug configure flags, how many calls per second can be made on variable values of $num for both?
> (I'd assume over twice as fast as calling both min/max from another function, but possibly slower than efficient_clamp, but haven't run this)
>
> For userland implementations that did use min/max, they probably weren't performance sensitive for the application.
>
> ```php
> function efficient_clamp(int|float $num, int|float $min, int|float $max): int|float {
> return $num < $min ? $min : ($num > $max ? $max : $num);
> }
> ```
Will combine both emails into one here. Will tackle your first point since that has to with performance as your second email also does.
Haven’t tested it with opcache enabled so cannot comment on the performance, but for my general testing the internal version is more than 50% faster than the min/max version. Slower than the “efficient” version in some cases and faster in others. That is to be expected though since the efficient version doesn’t check for a valid range, if a user mixes up the min and max values the efficient clamp function doesn’t work as intended and will always return either the min/max version and never the clamped value.
For your second point, that could be said for most internal functions, `str_contains`, `str_starts_with` and `str_ends_with` could all three be implemented in userland with customises parameter orders, as can most other internals function. Frankly I don’t see this as much of an objection or issue as a phrase used to gate keep what can become a new standardised internal function.
Thirdly, yes I will agree it’s inconsistent with min/max functions. That is mainly - as you stated, min/max takes mixed arguments and works with all comparable types. Though it technically isn’t inconsistent with most userland implementations as they mostly accept numerical values such as ints or floats into their clamp function, meaning only ints and floats are used anyway.
Though I could see a version of clamping being useful in the `DatePeriod` class to check if a `DateTime` is in range of a certain date range, but that’s OT for this RFC so.
As for your last and forth point, I’ll be updating the RFC with NaN behaviour, sticking with what Nikita suggested to throw when NaN is added to min or max, and return NaN is it’s added to value.
Thanks, Kim.
