Skip to content

Commit 1d56b2a

Browse files
pangeorgskyoxZkasperk81
authored
Improve ArgumentOutOfRange exception in Math.Clamp (#118824)
* Improve ArgumentOutOfRange exception in Math.Clamp Replaces e.g. "System.ArgumentException: '0' cannot be greater than -2." with System.ArgumentException: 'minValue = 0' cannot be greater than maxValue = -2 This follows the behavior of System.Math.Random Fix #115337 * Update src/libraries/System.Private.CoreLib/src/System/Math.cs Co-authored-by: skyoxZ <skyoxZ@qq.com> * Follow suggestions from GH comments and code quality rules * Apply the Math.cs change to BigInteger.Clamp * Use 'nameof' * Update src/libraries/System.Private.CoreLib/src/System/Math.cs Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com> * Applied suggestions from GH comments * Use ArgumentOutOfRangeException.ThrowLess in Math.cs and BigInteger.cs * Replace Math.ThrowMinMaxException with available method in ArgumentOutOfRangeException: ThrowIfLessThan * Add back Math.ThrowMinMaxException(min, max) This is to support throwing the error from INumber.Clamp as ArgumentOutOfRangeException.ThrowIfLessThan has a type constraint IComparable<T> so it cannot be used with nullable types. All other places use ArgumentOutOfRangeException.ThrowIfLessThan directly. * Fixed typo in Math.ThrowMinMaxException * Fixed tests for Math.Clamp Assert that Math.Clamp throws ArgumentOutOfRangeException * Fix the param name in the test so they pass * Remove Argument_MinMaxValue as it is no longer used --------- Co-authored-by: skyoxZ <skyoxZ@qq.com> Co-authored-by: = <=> Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com>
1 parent 19a23c8 commit 1d56b2a

File tree

9 files changed

+36
-108
lines changed

9 files changed

+36
-108
lines changed

‎src/libraries/System.Private.CoreLib/src/System/Double.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -940,20 +940,14 @@ bool IFloatingPoint<double>.TryWriteSignificandLittleEndian(Span<byte> destinati
940940
/// <inheritdoc cref="INumber{TSelf}.Clamp(TSelf, TSelf, TSelf)" />
941941
public static double Clamp(double value, double min, double max)
942942
{
943-
if (min > max)
944-
{
945-
Math.ThrowMinMaxException(min, max);
946-
}
943+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
947944
return Min(Max(value, min), max);
948945
}
949946

950947
/// <inheritdoc cref="INumber{TSelf}.ClampNative(TSelf, TSelf, TSelf)" />
951948
public static double ClampNative(double value, double min, double max)
952949
{
953-
if (min > max)
954-
{
955-
Math.ThrowMinMaxException(min, max);
956-
}
950+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
957951
return MinNative(MaxNative(value, min), max);
958952
}
959953

‎src/libraries/System.Private.CoreLib/src/System/Half.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,10 +1644,7 @@ public static int ILogB(Half x)
16441644
/// <inheritdoc cref="INumber{TSelf}.ClampNative(TSelf, TSelf, TSelf)" />
16451645
public static Half ClampNative(Half value, Half min, Half max)
16461646
{
1647-
if (min > max)
1648-
{
1649-
Math.ThrowMinMaxException(min, max);
1650-
}
1647+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
16511648
return MinNative(MaxNative(value, min), max);
16521649
}
16531650

‎src/libraries/System.Private.CoreLib/src/System/Int128.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,10 +1201,7 @@ public static Int128 BigMul(Int128 left, Int128 right, out Int128 lower)
12011201
/// <inheritdoc cref="INumber{TSelf}.Clamp(TSelf, TSelf, TSelf)" />
12021202
public static Int128 Clamp(Int128 value, Int128 min, Int128 max)
12031203
{
1204-
if (min > max)
1205-
{
1206-
Math.ThrowMinMaxException(min, max);
1207-
}
1204+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
12081205

12091206
if (value < min)
12101207
{

‎src/libraries/System.Private.CoreLib/src/System/Math.cs‎

Lines changed: 14 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,7 @@ public static decimal Ceiling(decimal d)
525525
[MethodImpl(MethodImplOptions.AggressiveInlining)]
526526
public static byte Clamp(byte value, byte min, byte max)
527527
{
528-
if (min > max)
529-
{
530-
ThrowMinMaxException(min, max);
531-
}
528+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
532529

533530
if (value < min)
534531
{
@@ -545,10 +542,7 @@ public static byte Clamp(byte value, byte min, byte max)
545542
[MethodImpl(MethodImplOptions.AggressiveInlining)]
546543
public static decimal Clamp(decimal value, decimal min, decimal max)
547544
{
548-
if (min > max)
549-
{
550-
ThrowMinMaxException(min, max);
551-
}
545+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
552546

553547
if (value < min)
554548
{
@@ -565,10 +559,7 @@ public static decimal Clamp(decimal value, decimal min, decimal max)
565559
[MethodImpl(MethodImplOptions.AggressiveInlining)]
566560
public static double Clamp(double value, double min, double max)
567561
{
568-
if (min > max)
569-
{
570-
ThrowMinMaxException(min, max);
571-
}
562+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
572563

573564
if (value < min)
574565
{
@@ -585,10 +576,7 @@ public static double Clamp(double value, double min, double max)
585576
[MethodImpl(MethodImplOptions.AggressiveInlining)]
586577
public static short Clamp(short value, short min, short max)
587578
{
588-
if (min > max)
589-
{
590-
ThrowMinMaxException(min, max);
591-
}
579+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
592580

593581
if (value < min)
594582
{
@@ -605,10 +593,7 @@ public static short Clamp(short value, short min, short max)
605593
[MethodImpl(MethodImplOptions.AggressiveInlining)]
606594
public static int Clamp(int value, int min, int max)
607595
{
608-
if (min > max)
609-
{
610-
ThrowMinMaxException(min, max);
611-
}
596+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
612597

613598
if (value < min)
614599
{
@@ -625,10 +610,7 @@ public static int Clamp(int value, int min, int max)
625610
[MethodImpl(MethodImplOptions.AggressiveInlining)]
626611
public static long Clamp(long value, long min, long max)
627612
{
628-
if (min > max)
629-
{
630-
ThrowMinMaxException(min, max);
631-
}
613+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
632614

633615
if (value < min)
634616
{
@@ -660,10 +642,7 @@ public static long Clamp(long value, long min, long max)
660642
[MethodImpl(MethodImplOptions.AggressiveInlining)]
661643
public static nint Clamp(nint value, nint min, nint max)
662644
{
663-
if (min > max)
664-
{
665-
ThrowMinMaxException(min, max);
666-
}
645+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
667646

668647
if (value < min)
669648
{
@@ -681,10 +660,7 @@ public static nint Clamp(nint value, nint min, nint max)
681660
[MethodImpl(MethodImplOptions.AggressiveInlining)]
682661
public static sbyte Clamp(sbyte value, sbyte min, sbyte max)
683662
{
684-
if (min > max)
685-
{
686-
ThrowMinMaxException(min, max);
687-
}
663+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
688664

689665
if (value < min)
690666
{
@@ -701,10 +677,7 @@ public static sbyte Clamp(sbyte value, sbyte min, sbyte max)
701677
[MethodImpl(MethodImplOptions.AggressiveInlining)]
702678
public static float Clamp(float value, float min, float max)
703679
{
704-
if (min > max)
705-
{
706-
ThrowMinMaxException(min, max);
707-
}
680+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
708681

709682
if (value < min)
710683
{
@@ -722,10 +695,7 @@ public static float Clamp(float value, float min, float max)
722695
[MethodImpl(MethodImplOptions.AggressiveInlining)]
723696
public static ushort Clamp(ushort value, ushort min, ushort max)
724697
{
725-
if (min > max)
726-
{
727-
ThrowMinMaxException(min, max);
728-
}
698+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
729699

730700
if (value < min)
731701
{
@@ -743,10 +713,7 @@ public static ushort Clamp(ushort value, ushort min, ushort max)
743713
[CLSCompliant(false)]
744714
public static uint Clamp(uint value, uint min, uint max)
745715
{
746-
if (min > max)
747-
{
748-
ThrowMinMaxException(min, max);
749-
}
716+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
750717

751718
if (value < min)
752719
{
@@ -764,10 +731,7 @@ public static uint Clamp(uint value, uint min, uint max)
764731
[CLSCompliant(false)]
765732
public static ulong Clamp(ulong value, ulong min, ulong max)
766733
{
767-
if (min > max)
768-
{
769-
ThrowMinMaxException(min, max);
770-
}
734+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
771735

772736
if (value < min)
773737
{
@@ -800,10 +764,7 @@ public static ulong Clamp(ulong value, ulong min, ulong max)
800764
[CLSCompliant(false)]
801765
public static nuint Clamp(nuint value, nuint min, nuint max)
802766
{
803-
if (min > max)
804-
{
805-
ThrowMinMaxException(min, max);
806-
}
767+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
807768

808769
if (value < min)
809770
{
@@ -1496,7 +1457,7 @@ public static unsafe double Truncate(double d)
14961457
[DoesNotReturn]
14971458
internal static void ThrowMinMaxException<T>(T min, T max)
14981459
{
1499-
throw new ArgumentException(SR.Format(SR.Argument_MinMaxValue, min, max));
1460+
throw new ArgumentOutOfRangeException(nameof(max), max, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, nameof(max), max, min));
15001461
}
15011462

15021463
public static double ScaleB(double x, int n)

‎src/libraries/System.Private.CoreLib/src/System/Single.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -937,20 +937,14 @@ bool IFloatingPoint<float>.TryWriteSignificandLittleEndian(Span<byte> destinatio
937937
/// <inheritdoc cref="INumber{TSelf}.Clamp(TSelf, TSelf, TSelf)" />
938938
public static float Clamp(float value, float min, float max)
939939
{
940-
if (min > max)
941-
{
942-
Math.ThrowMinMaxException(min, max);
943-
}
940+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
944941
return Min(Max(value, min), max);
945942
}
946943

947944
/// <inheritdoc cref="INumber{TSelf}.ClampNative(TSelf, TSelf, TSelf)" />
948945
public static float ClampNative(float value, float min, float max)
949946
{
950-
if (min > max)
951-
{
952-
Math.ThrowMinMaxException(min, max);
953-
}
947+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
954948
return MinNative(MaxNative(value, min), max);
955949
}
956950

‎src/libraries/System.Private.CoreLib/src/System/UInt128.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,10 +1399,7 @@ public static UInt128 BigMul(UInt128 left, UInt128 right, out UInt128 lower)
13991399
/// <inheritdoc cref="INumber{TSelf}.Clamp(TSelf, TSelf, TSelf)" />
14001400
public static UInt128 Clamp(UInt128 value, UInt128 min, UInt128 max)
14011401
{
1402-
if (min > max)
1403-
{
1404-
Math.ThrowMinMaxException(min, max);
1405-
}
1402+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
14061403

14071404
if (value < min)
14081405
{

‎src/libraries/System.Runtime.Numerics/src/Resources/Strings.resx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@
6969
<data name="Argument_InvalidHexStyle" xml:space="preserve">
7070
<value>With the AllowHexSpecifier bit set in the enum bit field, the only other valid bits that can be combined into the enum value must be a subset of those in HexNumber.</value>
7171
</data>
72-
<data name="Argument_MinMaxValue" xml:space="preserve">
73-
<value>'{0}' cannot be greater than {1}.</value>
74-
</data>
7572
<data name="Argument_MustBeBigInt" xml:space="preserve">
7673
<value>The parameter must be a BigInteger.</value>
7774
</data>

‎src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3947,10 +3947,7 @@ public static BigInteger Clamp(BigInteger value, BigInteger min, BigInteger max)
39473947
min.AssertValid();
39483948
max.AssertValid();
39493949

3950-
if (min > max)
3951-
{
3952-
ThrowMinMaxException(min, max);
3953-
}
3950+
ArgumentOutOfRangeException.ThrowIfLessThan(max, min);
39543951

39553952
if (value < min)
39563953
{
@@ -3962,12 +3959,6 @@ public static BigInteger Clamp(BigInteger value, BigInteger min, BigInteger max)
39623959
}
39633960

39643961
return value;
3965-
3966-
[DoesNotReturn]
3967-
static void ThrowMinMaxException<T>(T min, T max)
3968-
{
3969-
throw new ArgumentException(SR.Format(SR.Argument_MinMaxValue, min, max));
3970-
}
39713962
}
39723963

39733964
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />

‎src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/Math.cs‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2212,20 +2212,20 @@ public static void Clamp_Decimal(decimal value, decimal min, decimal max, decima
22122212
}
22132213

22142214
[Fact]
2215-
public static void Clamp_MinGreaterThanMax_ThrowsArgumentException()
2216-
{
2217-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((sbyte)1, (sbyte)2, (sbyte)1));
2218-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((byte)1, (byte)2, (byte)1));
2219-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((short)1, (short)2, (short)1));
2220-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((ushort)1, (ushort)2, (ushort)1));
2221-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((int)1, (int)2, (int)1));
2222-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((uint)1, (uint)2, (uint)1));
2223-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((long)1, (long)2, (long)1));
2224-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((ulong)1, (ulong)2, (ulong)1));
2225-
2226-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((float)1, (float)2, (float)1));
2227-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((double)1, (double)2, (double)1));
2228-
AssertExtensions.Throws<ArgumentException>(null, () => Math.Clamp((decimal)1, (decimal)2, (decimal)1));
2215+
public static void Clamp_MinGreaterThanMax_ThrowsArgumentOutOfRangeException()
2216+
{
2217+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((sbyte)1, (sbyte)2, (sbyte)1));
2218+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((byte)1, (byte)2, (byte)1));
2219+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((short)1, (short)2, (short)1));
2220+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((ushort)1, (ushort)2, (ushort)1));
2221+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((int)1, (int)2, (int)1));
2222+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((uint)1, (uint)2, (uint)1));
2223+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((long)1, (long)2, (long)1));
2224+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((ulong)1, (ulong)2, (ulong)1));
2225+
2226+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((float)1, (float)2, (float)1));
2227+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((double)1, (double)2, (double)1));
2228+
AssertExtensions.Throws<ArgumentOutOfRangeException>("max", () => Math.Clamp((decimal)1, (decimal)2, (decimal)1));
22292229
}
22302230

22312231
[Theory]

0 commit comments

Comments
 (0)