RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
Karl T
duke at openjdk.org
Tue Feb 14 20:00:54 UTC 2023
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev <tvaleev at openjdk.org> wrote:
>> clamp() methods added to Math and StrictMath
>>
>> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` value and safely clamps it to an `int` range. Other overloads work with a particular type (long, float and double). Using similar approach in other cases (e.g. `float clamp(double, float, float)`) may cause accidental precision loss even if the value is within range, so I decided to avoid this.
>>
>> In all cases, `max >= min` precondition should met. For double and float we additionally order `-0.0 < 0.0`, similarly to what Math.max or Double.compare do. In double and float overloads I try to keep at most one arg-check comparison on common path, so the order of checks might look unusual.
>>
>> For tests, I noticed that tests in java/lang/Math don't use any testing framework (even newer tests), so I somehow mimic the approach of neighbour tests.
>
> Tagir F. Valeev has updated the pull request incrementally with one additional commit since the last revision:
>
> Comments in tests
Regarding the asymmetric `int clamp( long value, int min, int max )` method:
I understand the idea behind using `long`, but IMHO there is a good reason to use `int`instead.
When using the `var` keyword, it could easy result in unwanted type change from `long` to `int`.
E.g. following simple code unexpectedly changes the type from `long` to `int`:
~~~java
var longValue = 1234L;
var intValue = Math.clamp( longValue, 0, 100 );
~~~
When doing the same with `Math.min()` and `max()`, it does **not change type**:
~~~java
var longValue2 = Math.min( 100, Math.max( longValue, 0 ) );
~~~
If you don't want that type change, it is required to pass a long min or max value:
~~~java
var longValue = 1234L;
var longValue2 = Math.clamp( longValue, 0L, 100 );
~~~
This is confusing, isn't it?
The idea behind using `long` as first parameter is "This allows to use method to safely cast long value to int" (from javadoc).
But if changing to `int clamp( int value, int min, int max )`, safely casting long to int is still possible by passing int values to min and max an casting the result to int. E.g.:
~~~java
int a = (int) Math.clamp( longValue, 0, 100 );
~~~
Also not sure whether the idea behind clamp is to provide a mechanism for safe casting...
IMHO the benefit of "safe casting long to int" is very low compared to the risk of unwanted type change when using `var` keyword.
-------------
PR: https://git.openjdk.org/jdk/pull/12428
More information about the core-libs-dev
mailing list