runtime: [API Proposal]: Throw helpers should return the valid value back
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.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 17 (10 by maintainers)
The inability to write arbitrary code in base/this delegation is an issue well beyond ThrowIfNull argument validation and deserves separate language consideration rather than contorting such an API to address the lack of such support.
The API would also need to be generic to return a strongly-typed reference usable in the same manner as the original, and that brings with it its own complexity and cost, plus there are a fair number of devs and analyzers that would consider such a return value (frequently ignored) to be a smell.
We’ll need to agree to disagree.
I think it’s in the eye of the beholder. I think the approach of validating and then storing is perfectly reasonable and is no more or less unified than the proposal.
My opinion is it’s not worth adding a new method with a new name just to be able to write:
instead of:
It’s still valid to write:
if that’s your preference.
Literally nothing can be changed around the IL semantics. The body of callee is agnostic about caller, and IL doesn’t know anything about inlining.
.NET languages especially C# won’t transform public API shape. The compiler would not emit anything public that’s not declared by code (except record). Additionally, there’s no situation for compiler to detect.
This is possible. But I didn’t find anyone interested in this from the original discussion #48573. Anyway, the existing throw helper can’t change in any aspect. The newly proposed one won’t be tight with its characteristics.