runtime: [API Proposal]: ArgumentOutOfRangeException helper methods
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 baseArgumentException
. 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 #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 inArgumentException
. That’s break all tests withAssert.Throw(paramName, () => {})
because they’re strictly comparing parameter name andParamName
property. I don’t think this is a problem for end users, but replacement in /runtime could be tricky Discussed in #69637 andThrowIf
should contains additional argument for passing custom message. Should it be juststring
orInterpolatedStringHandler
?ThrowIf
completely removed from proposal.UPD: replaced byThrowIfLessThan
and others similar methods withIComparisonOperators
constraint doesn’t work for TimeSpan, DateTime, etc. Related to #67744, but Idk why these types can’t implement IComparisonOperators that doesn’t cause any overflowing issues. Could be replaced by IComparable<T>?IComparable<T>
ThrowIfNegative{OrZero}
requires too much for custom types due toIBaseNumber<>
constraint (we only need T.Zero). I think most of the users will useThrowIfLess
with manual passing “zero” value instead (onlyIComparable<T>
required for this)- I saw a lot of similar conditions with throwing base
ArgumentException
insteadArgumentOutOfRangeException
. Should we duplicate these methods for it? - #70515
- Renaming
ThrowIfNotBetween
->ThrowIfOutsideRange
https://github.com/dotnet/runtime/pull/69767#discussion_r906962047
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 10
- Comments: 31 (30 by maintainers)
This is an issue where we need high confidence that it’s going to address most of our own usage; otherwise, we’ll take it through API review and either there will be a bunch of questions about what’s missing, or it’ll get approved and then when we go to use it we might find a bunch of issues, and we’ll need to flip it back to unapproved to take it back through again.
As such, my preference is that the proposal actually include doing the work to roll it out across dotnet/runtime, so that we can fully understand what issues there might be / what might be missing. That work will be necessary anyway, so it’s better to just front-load it.
Obviously some things might need to be tweaked post API-review, but hopefully it’d mostly be search-and-replace for things like naming.
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.
Yes, you can assign this one on me I’ll also look at
ThrowIfNotBetween
usages for some patterns exceptThrowIfNotInRange(index, ICollection)
I’m enthused about this too. We’re working through some planning tasks right now, and as part of that we’re crafting our “work planned” epics across our areas. I’m going to mark this as 8.0.0 so that we can include this and commit to refining these if/as needed and getting the approved set into an early preview of .NET 8.
I was just looking for a throw helper that would help in the Console App scenario where I want to ensure I received the expected number of arguments.
ArgumentOutOfRangeException.ThrowIfLessThan
would be useful for that. Before I reached for that though, I was also curious ifArgumentException.ThrowIfNullOrEmpty([NotNull] string?[]? argument, ...)
existed (it doesn’t).It’ll depend on whether the message contains any meaningful additional information. In the example you reference, the message is just “StartIndex cannot be less than zero”, which doesn’t provide anything useful beyond the default message would for a ThrowIfNegative method, so this case could be changed. Thanks.