[API Proposal]: ArgumentOutOfRangeException helper methods

[hrrrrustic]

UPD: Fully rewrote description after creating PR
UPD2: Initially IComparisonOperators<T, T> was used instead of IComparable<T>
Proposed methods (split them as mush as possible) after looking at /runtime code:
(Not sure about struct constraint, everything looks working without it, but it exists in Linq.Max/Min)

namespace System;

public class ArgumentOutOfRangeException : ArgumentException
{
      public static void ThrowIfZero<T>(T value, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, INumberBase<T>, IComparable<T>;
      public static void ThrowIfNegative<T>(T value, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, INumberBase<T>, ISignedNumber<T>, IComparable<T>;
      public static void ThrowIfNegativeOrZero<T>(T value, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, INumberBase<T>, ISignedNumber<T>, IComparable<T>;
      
      public static void ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, IComparable<T>;
      public static void ThrowIfGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, IComparable<T>;
      
      public static void ThrowIfLessThan<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null)
          where T : struct, IComparable<T>;
      public static void ThrowIfLessThanOrEqual<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null) 
          where T : struct, IComparable<T>;
      
      public static void ThrowIfNotBetween<T>(T value, T inclusiveLower, T inclusiveUpper, [CallerArgumentExpression("value")] string? paramName = null)
          where T : struct, IComparable<T>;
}

Some statistics

Actual usages count will be a little bit lower since some of them are under libraries with old targets (net 4.x or netstandard)

• ThrowIfNull
  Usages: <10
  I didn't split this in PR, but saw a few of them with ArgumentOutOfRangeException, InvalidArgumentException and base ArgumentException. It probably makes sense to add this one for better null analysis
• ThrowIfZero
  I personally don't think that this one is good enough, but there is a [API Proposal]: Add ArgumentOutOfRangeException.ThrowIfZero() #68339
  Usages: 8
• ThrowIfNegative
  Usages: 755
• ThrowIfNegativeOrZero
  Usages: 100
• ThrowIfGreaterThan
  Usages: 204
• ThrowIfGreaterThanOrEqual
  Usages: 32
• ThrowIfLessThan
  Usages: 174
• ThrowIfLessThanOrEqual
  Usages: 4
• ThrowIfNotBetween
  Usages: 255

Risks & Problems & Questions

• Using casts while passing argument to helper will cause saving it to ParamName property in ArgumentException. That's break all tests with Assert.Throw(paramName, () => {}) because they're strictly comparing parameter name and ParamName property. I don't think this is a problem for end users, but replacement in /runtime could be tricky
• ThrowIf should contains additional argument for passing custom message. Should it be just string or InterpolatedStringHandler? Discussed in [API Proposal]: More helper methods for exception types #69637 and ThrowIf completely removed from proposal.
• ThrowIfLessThan and others similar methods with IComparisonOperators constraint doesn't work for TimeSpan, DateTime, etc. Related to Determine recommended behavior of checked/unchecked operators for DateTime and similar types #67744, but Idk why these types can't implement IComparisonOperators that doesn't cause any overflowing issues. Could be replaced by IComparable<T>? UPD: replaced by IComparable<T>
• ThrowIfNegative{OrZero} requires too much for custom types due to IBaseNumber<> constraint (we only need T.Zero). I think most of the users will use ThrowIfLess with manual passing "zero" value instead (only IComparable<T> required for this)
• I saw a lot of similar conditions with throwing base ArgumentException instead ArgumentOutOfRangeException. Should we duplicate these methods for it?
• [API Proposal]: Throw helpers should return the valid value back #70515
• Renaming ThrowIfNotBetween -> ThrowIfOutsideRange ArgumentOutOfRangeException throw helpers #69767 (comment)

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[stephentoub]

Some optimization hacks like uint casts can be lost by using this API

@tannergooding, were you looking to add an IsBetween method to one of the generic math interfaces?

[stephentoub]

Methods in the second group needs more information for throwing (at least message arg). But I'm not sure should it be just a string or interpolated handler

Can you elaborate? Do you mean the message needs to include information about the bounds being checked?

[hrrrrustic]

Methods in the second group needs more information for throwing (at least message arg). But I'm not sure should it be just a string or interpolated handler

Can you elaborate? Do you mean the message needs to include information about the bounds being checked?

Yes. Most of the usages in dotnet/runtime use prepared messages from SR and can be just passed as one more string param

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs

Line 131 in 6ca8c9b

throw new ArgumentOutOfRangeException(nameof(value), SR.net_io_timeout_use_gt_zero);

But I think it is fine to do something like this sometimes

if (arg < someMinValue) throw ArgumentOutOfRangeException(nameof(arg), $"value has to be at least {someMinValue})

and in the new API laziness of interpolation will be lost without interpolation handler

[stephentoub]

These helper APIs are meant for the most common cases where the caller wants to use a standard error message. I don't think these APIs should accept a message; the implementation can use the bounds it's given to create the message, but the user shouldn't be able to customize it. At that point, they can just write their own check.

[tannergooding]

were you looking to add an IsBetween method to one of the generic math interfaces?

Yep. I'll be getting a proposal up later today most likely.

[tannergooding]

ThrowIfNotPositive

This one needs a different name. In the realm of generic math, the IEEE 754 (floating-point) spec, and a few other cases 0 is considered positive and INumberBase.IsPositive will return true. Likewise for -0 it will return false and IsNegative will return true.

[tannergooding]

I'd likely recommend the other APIs have slightly different names as well:

• ThrowIfNegative - LGTM
• ThrowIfNotPositive - Needs a different name, as per above
• ThrowIfGreater - Should probably be ThrowIfGreaterThan
• ThrowIfLower - Should probably be ThrowIfLessThan
• ThrowIfNotInRange - Should probably be ThrowIfNotBetween

*GreaterThan(x, y) is the name we use in the vector types, LINQ expressions, and various other places. *LessThan is then its counterpart.

IsBetween is the name we took last week for some other APIs and is what I'll be proposing for INumber.IsBetween, so IfNotBetween seems like the opposite of that.

[hrrrrustic]

Agree, but not sure how can ThrowIfNotPositive be renamed, I thought above ThrowIfNegativeOrZero but is still falls in -0and +0 😄

Updated proposal for other methods.

[tannergooding]

ThrowIfNegativeOrZero seems reasonable to me. -0 is negative and/or zero, depending on how you look. +0 is zero. So both will cause a throw and only positive/non-zero values will "succeed".

This also "fits" the naming convention we'd presumably do if we had ThrowIfGreaterThanOrEqual and ThrowIfLessThanOrEqual methods.

[hrrrrustic]

Uhh, make sense, updated proposal

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

Similar to #62628, #58047 and #51700 (also #68339)

Tried to collect all possible helpers for ArgumentOutOfRangeException in this issue as per

We've said we're ok with such helpers in general (not this specific proposal), as long as they're common use cases (there are an infinite number of methods we'd have to add if we wanted to handle all of the argument validation anyone does today). In particular, we've talked about someone needing to come up with a proposal for what the helpers should be for ArgumentOutOfRangeException in order to cover the 95% of use for it across dotnet/runtime.

Originally posted by @stephentoub in #68339 (comment)

API Proposal

All methods below are inside ArgumentOutOfRangeException

Group 1:

public static void ThrowIfNegative<T>(T value, [CallerArgumentExpression("value")] string? paramName = null)
    where T: ISignedNumber<T> {}

public static void ThrowIfNegativeOrZero<T>(T value, [CallerArgumentExpression("value")] string? paramName = null)
    where T: INumber<T> {}

Group 2:

// Note that there are no 'INumber<T>' constraint! This methods can be used for any types, not only for numbers.
// Also possible to extend usages for user defined types by implementing 'IComparisonOperators'

public static void ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null)
    where T: IComparisonOperators<T, T> {}

public static void ThrowIfLowerThan<T>(T value, T other, [CallerArgumentExpression("value")] string? paramName = null)
    where T: IComparisonOperators<T, T> {}

public static void ThrowIfNotBetween<T>(T value, T left, T right, [CallerArgumentExpression("value")] string? paramName = null) 
    where T: IComparisonOperators<T, T> {}

API Usage

Group 1:

ThrowIfNegative is obviously the most usable helper that will cover most of the boring if with validation input args. 😆

runtime/src/libraries/System.Private.CoreLib/src/System/Version.cs

Lines 28 to 39 in 6ca8c9b

	public Version(int major, int minor, int build, int revision)
	{
	if (major < 0)
	throw new ArgumentOutOfRangeException(nameof(major), SR.ArgumentOutOfRange_Version);
	if (minor < 0)
	throw new ArgumentOutOfRangeException(nameof(minor), SR.ArgumentOutOfRange_Version);
	if (build < 0)
	throw new ArgumentOutOfRangeException(nameof(build), SR.ArgumentOutOfRange_Version);
	if (revision < 0)

There is also kind of this helper in Random

runtime/src/libraries/System.Private.CoreLib/src/System/Random.cs

Lines 118 to 122 in 0c6d412

	public virtual long NextInt64(long maxValue)
	{
	if (maxValue < 0)
	{
	ThrowMaxValueMustBeNonNegative();

ThrowIfNegativeOrZero
The only difference from previous method that zero is also invalid value. There are just a few cases (found them by regex <= 0.+throw) for it in dotnet/runtime, but I believe this could also make sense

runtime/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/IntegerValidator.cs

Lines 21 to 23 in 0c6d412

	public IntegerValidator(int minValue, int maxValue, bool rangeIsExclusive, int resolution)
	{
	if (resolution <= 0) throw new ArgumentOutOfRangeException(nameof(resolution));

Group 2:
ThrowIfGreaterThan and ThrowIfLowerThan has two types of usage:

• with constant (rare for /runtime, but might be useful to user code for custom types max/min values e.g. version)
  

  runtime/src/libraries/System.Private.CoreLib/src/System/Decimal.cs

  Lines 308 to 311 in c0c614c

  	public Decimal(int lo, int mid, int hi, bool isNegative, byte scale)
  	{
  	if (scale > 28)
  	throw new ArgumentOutOfRangeException(nameof(scale), SR.ArgumentOutOfRange_DecimalScale);

• with collection lengths (common for /runtime)
  

  runtime/src/libraries/System.Private.CoreLib/src/System/String.cs

  Lines 94 to 95 in 6ca8c9b

  	if (startIndex > value.Length - length)
  	throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);

ThrowIfNotBetween commonly used in some inserting/coping operation to confirm that passed index > 0 and < length

runtime/src/libraries/System.Collections/src/System/Collections/Generic/SortedList.cs

Lines 446 to 448 in 6ca8c9b

	if (arrayIndex < 0 || arrayIndex > array.Length)
	{
	throw new ArgumentOutOfRangeException(nameof(arrayIndex), arrayIndex, SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);

Alternative Designs

1. Additional overloads for IFloatingPoint with epsilon arg
2. Some naming changes like
   * ThrowIfNotPositive -> ThrowIfNegativeOrZero
   * ThrowIfNotInRange -> ThrowIfOutsideRange
   * ThrowIfGreater -> ThrowIf{Larger/Bigger} and ThrowIfLower -> ThrowIfSmaller

Risks

• Some optimization hacks like uint casts can be lost by using this API
  UPD: Shoud be fixed with IsBetween in generic math

• Methods in the second group needs more information for throwing (at least message arg). But I'm not sure should it be just a string or interpolated handler
  UPD: Decided that for most cases default messages will be enough, otherwise user should use old style

• This code compiles

  public void Test(uint a)
  {
      if (a > 1)
          throw new ArgumentOutOfRangeException();
  }

  But this (without u suffix) not

  public void Test(uint a)
  {
      MyArgumentOutOfRangeException.ThrowIfGreater(a, 1);
  }

  Same for ulong and coming uint128

Author:	hrrrrustic
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-