-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
Background and motivation
I've been using the newly introduced ArgumentNullException.ThrowIfNull method quite a bit to simplify my code and reduce the reliance on nameof for passing the argument name.
However, I'm running into the following situation a lot:
public SomeClass(string value)
{
ArgumentNullException.ThrowIfNull(value);
this.value = value;
}Most often, this happens on constructors, where we want to validate the value and then assign it to a field in the class.
Prior to the helper method, I used to use throw expressions for this:
public SomeClass(string value)
{
this.value = value ?? throw new ArgumentNullException(nameof(value));
}I think code would be a lot simpler if we could combine both of these like this:
public SomeClass(string value)
{
this.value = ArgumentNullException.ThrowIfNull(value);
}By making it so that the exception validation helpers always returned the value on exit (instead of being void methods.
This is how most exception guard classes/libraries work, exactly because one of the main cases is to assign the "validated" value to some backing field after validation happens.
The "validate and return" pattern guarantees I'll only ever need to use one line to initialize any value and that there will be no need to repeat the value multiple times (once on validation, and another time on assignment).
This is also particularly important when dealing with base classes, where we need inline validation to happen but still pass the value forward:
public abstract Base
{
public Base(string value)
{
ArgumentNullException.ThrowIfNull(value);
this.value = value;
}
}
public class Inheritor : Base
{
public Inheritor(string value)
// : base(ArgumentNullException.ThrowIfNull(value)) <- this won't work
: base(value ?? throw new ArgumentNullException(nameof(value))) // <- fallback to throw expressions :(
{
}
}API Proposal
Current:
public class ArgumentNullException : ArgumentException
{
public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression("argument")] string? paramName = null);
}New:
public class ArgumentNullException : ArgumentException
{
public static T ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
public static T ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
where T : struct
}This would apply to other such helpers, like ArgumentException.ThrowIfNullOrEmpty proposed here:
As well as other proposals such as the ones for ArgumentOutOfRangeException here:
API Usage
public SomeClass(string reference, int? value)
{
this.reference = ArgumentNullException.ThrowIfNull(reference);
this.value = ArgumentNullException.ThrowIfNull(value);
}Alternative Designs
If I was designing this from scratch, I'd rather have these methods named like this:
ArgumentNullException.EnsureNotNull(value)As "Ensure{ConditionThatIDon'tWant}" makes more sense when a value is returned, and is also pretty common among existing guard classes and libraries.
It is also fairly well known that an "Ensure" method is expected to throw when the validation "fails".
Risks
I think my original proposal is fairly risk-free, but renaming the methods from ThrowIfX to EnsureNotX (or EnsureY when Y is the opposite of X) would obviously be a nasty breaking change.