[API Proposal]: ArgumentException.ThrowIf method

[DrkWzrd]

Background and motivation

We are seeing the creation of many helper methods to throw argument exceptions in a clean, one-liner way. But I feel it lacks one core method. The ability to throw argument exception by some condition.

Check the difference in readability and "bloat"

//current
public ExampleConstructor(int integer, string? str, char chr, Something? something, IService? service)
{
    ArgumentOutOfRangeException.ThrowIfGreaterThan(integer, int.MaxValue / 2);
    ArgumentException.ThrowIfNullOrWhiteSpace(str);
    if (str.Equals("bad"))
    {
        throw new ArgumentException("This is a bad string", nameof(str));
    }
    if(char.IsLetter(chr))
    {
        throw new ArgumentException("Can not be a letter", nameof(chr));
    }
    ArgumentNullException.ThrowIfNull(something);
    if (!something.CheckState)
    {
        throw new ArgumentException("Something, invalid state", nameof(something));
    }
    ArgumentNullException.ThrowIfNull(service);
    if (!service.IsInitialized)
    {
        throw new ArgumentException("Service not initialized", nameof(service));
    }

    //Do things
}

//proposed
public ExampleConstructor(int integer, string? str, char chr, Something? something, IService? service)
{
    ArgumentOutOfRangeException.ThrowIfGreaterThan(integer, int.MaxValue / 2);

    ArgumentException.ThrowIfNullOrWhiteSpace(str);
    ArgumentException.ThrowIf(IsBadString, str, "This is a bad string");

    ArgumentException.ThrowIf(char.IsLetter, chr, "Can not be a letter");

    ArgumentNullException.ThrowIfNull(something);
    ArgumentException.ThrowIf(StateIsInvalid, something, "Something, invalid state");

    ArgumentNullException.ThrowIfNull(service);
    ArgumentException.ThrowIf(NotInitialized, service, "Service not initialized");
    
    //Do things
}

private /*static*/ bool IsBadString(string str) => str.Equals("bad");
private /*static*/ bool StateIsInvalid(Something smt) => !smt.CheckState;
private /*static*/ bool NotInitilized(IService svc) => !svc.IsInitialized;

And this can be used successfully to check things about IEnumerable (IsEmpty?, IsCountValid? AnyHasInvalidThings?, etc...)

API Proposal

namespace System.Collections.Generic;

public class ArgumentException
{
    public static void ThrowIf<T>(Func<T, bool> condition, T argument, string? message = null, [CallerArgumentExpression("argument")] string? paramName = null);
}

API Usage

The example above.

Alternative Designs

None

Risks

I don't see any, except another method to mantain (but is a tiny method ;) )

[huoyaoyuan]

The lambda is going to add significant overhead. Argument validation is typically expected to be more light-weighted.
The main benefits of the throw helper method are performance optimization and enabling CallerArgumentExpression.

If there's must-inline lambda like F#, this will be more acceptable.

[MichalPetryka]

The lambda is going to add significant overhead.

It would probably not be the case with #85014 fixed and #85197 being revived.

[DrkWzrd]

The lambda is going to add significant overhead. Argument validation is typically expected to be more light-weighted. The main benefits of the throw helper method are performance optimization and enabling CallerArgumentExpression.

If there's must-inline lambda like F#, this will be more acceptable.

Sorry, I don't have that level of proficiency in .net, feel free to indicate me what performance-oriented changes should be done.

[colejohnson66]

The proper method would just be passing a boolean straight in. There's no reason to pass an immediately invoked delegate if one can just pass the result. Sure, there's no logical difference (program will execute the same) between these two blocks of code:

void Test1(string str) {
    ArgumentException.ThrowIf(x => x != "expected", str);
}

void Test2(string str) {
    ArgumentException.ThrowIf(str != "expected", str);
}

But the former (Test1) will allocate a new delegate on the heap, which will then have to be invoked. Eventually, the garbage collector will get involved as well. All that is overhead that we want to avoid.

The "correct" API would look like this:

namespace System.Collections.Generic;

public class ArgumentException
{
    public static void ThrowIf<T>(bool condition, T argument, string? message = null, [CallerArgumentExpression("argument")] string? paramName = null);
}

---

Tangent: if your use case is to validate an IEnumerable before actually using it, you should consider alternative methods. Multiple enumeration is a big no-no. Essentially:

// bad
void Test<T>(IEnumerable<T> source)
{
    ArgumentException.ThrowIf(!source.Any(), source);
 
    // this will cause multiple enumeration which incur performance issues or even exceptions
    foreach (T element in source)
        Use(element);
}

// good
void Test<T>(IEnumerable<T> source)
{
    using IEnumerator<T> e = source.GetEnumerator();
    if (!e.MoveNext())
        throw new InvalidOperationException("Source is empty.");

    do
    {
        Use(e.Current);
    } while (e.MoveNext());
}

[stephentoub]

The "correct" API would look like this:

But even then, this requires producing the message before the condition is checked, and that itself frequently has overheads, eg reading a string from a resx, or string.Fornat'ing a message.

This has been discussed before in various forms. We have no plans to add such an overload. Just do if (!condition) throw new ArgumentException(...).

[ghost]

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

We are seeing the creation of many helper methods to throw argument exceptions in a clean, one-liner way. But I feel it lacks one core method. The ability to throw argument exception by some condition.

Check the difference in readability and "bloat"

//current
public ExampleConstructor(int integer, string? str, char chr, Something? something, IService? service)
{
    ArgumentOutOfRangeException.ThrowIfGreaterThan(integer, int.MaxValue / 2);
    ArgumentException.ThrowIfNullOrWhiteSpace(str);
    if (str.Equals("bad"))
    {
        throw new ArgumentException("This is a bad string", nameof(str));
    }
    if(char.IsLetter(chr))
    {
        throw new ArgumentException("Can not be a letter", nameof(chr));
    }
    ArgumentNullException.ThrowIfNull(something);
    if (!something.CheckState)
    {
        throw new ArgumentException("Something, invalid state", nameof(something));
    }
    ArgumentNullException.ThrowIfNull(service);
    if (!service.IsInitialized)
    {
        throw new ArgumentException("Service not initialized", nameof(service));
    }

    //Do things
}

//proposed
public ExampleConstructor(int integer, string? str, char chr, Something? something, IService? service)
{
    ArgumentOutOfRangeException.ThrowIfGreaterThan(integer, int.MaxValue / 2);

    ArgumentException.ThrowIfNullOrWhiteSpace(str);
    ArgumentException.ThrowIf(IsBadString, str, "This is a bad string");

    ArgumentException.ThrowIf(char.IsLetter, chr, "Can not be a letter");

    ArgumentNullException.ThrowIfNull(something);
    ArgumentException.ThrowIf(StateIsInvalid, something, "Something, invalid state");

    ArgumentNullException.ThrowIfNull(service);
    ArgumentException.ThrowIf(NotInitialized, service, "Service not initialized");
    
    //Do things
}

private /*static*/ bool IsBadString(string str) => str.Equals("bad");
private /*static*/ bool StateIsInvalid(Something smt) => !smt.CheckState;
private /*static*/ bool NotInitilized(IService svc) => !svc.IsInitialized;

And this can be used successfully to check things about IEnumerable (IsEmpty?, IsCountValid? AnyHasInvalidThings?, etc...)

API Proposal

namespace System.Collections.Generic;

public class ArgumentException
{
    public static void ThrowIf<T>(Func<T, bool> condition, T argument, string? message = null, [CallerArgumentExpression("argument")] string? paramName = null);
}

API Usage

The example above.

Alternative Designs

None

Risks

I don't see any, except another method to mantain (but is a tiny method ;) )

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