[API Proposal]: Generic ArgumentNullException/ArgumentException that returns original value when not null

[techfan101]

Background and motivation

In many cases, I find that I check the validity of an argument with the sole intent of assigning it to a local variable or automatic property. With the current method signatures, this requires two lines, for example:

ArgumentNullException.ThrowIfNull(argument);
Argument = argument;

By adding a new overload, it would be possible to reduce clutter and perform both of these operations on a single line, for example:

Argument = ArgumentNullException.ThrowIfNull(argument);

This same issue exists in similar helper methods in the ArgumentException class. The addition of similar methods to the ArgumentOutOfRangeException class might also be helpful.

Alternatively, a single static class that combines common argument-checking scenarios might be preferable. I initially came here to champion overloading the existing class methods. However, after I completed the text for this issue, I became more convinced that the "Alternative Designs" discussed later are in fact a better approach.

API Proposal

namespace System;
...
public class ArgumentNullException : SystemException
{
  ...
  /// <summary>Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is null.</summary>
  /// <param name="argument">The reference type argument to validate as non-null.</param>
  /// <param name="paramName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
  [return: NotNull]
  public static T ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null);
  ...
}

Note: This is an example of a single argument-checking scenario. Additional scenarios are discussed in the "Alternative Designs" section.

API Usage

public class MyClass
{
  public MyClass(string? argument)
  {
    Argument = ArgumentNullException.ThrowIfNull(argument);
  }

  public string Argument { get; }
}

Alternative Designs

A new more-general helper class with static methods might be a preferable alternative to avoid the risk of signature ambiguity. I suggest the class should strive to achieve the following goals:

1. Use the System namespace to promote widespread use.
2. Have a short intuitive name to reduce clutter.
3. Limit the contents expressly to static methods that perform argument-checking.
4. Ensure that each static method accepts the widest possible range of appropriate types, using generics (constrained where appropriate).
5. Ensure that each static method either returns the original non-null argument value or throws an exception.
6. Mark similar existing methods (in ArgumentNullException and ArgumentException) as obsolete to promote uniform adoption of a more standardized mechanism.

I suggest the following minimal argument-checking scenarios should be considered:

1. Check if the argument is null (throws ArgumentNullException).
2. Check if the (string) argument is null or empty (throws ArgumentNullException/ArgumentException).
3. Check if the (string) argument is null, empty, or white space (throws ArgumentNullException/ArgumentException).
4. Check if the (comparable) argument is greater than or equal to a minimum value (throws ArgumentOutOfRangeException/ArgumentNullException).
5. Check if the (comparable) argument is less than or equal to the maximum value (throws ArgumentOutOfRangeException/ArgumentNullException).
6. Check if the (comparable) argument is within the specified inclusive minimum/maximum range (throws ArgumentOutOfRangeException/ArgumentNullException).

For what it's worth, in my own code, I've settled on the class name Guard and method names that closely resemble the existing method names (e.g. Guard.ThrowIfNull).

Finally, it's worth noting that inconsistent parameter ordering for the constructors of ArgumentNullException, ArgumentException, And ArgumentOutOfRangeException sometimes contributes to incorrect error reporting. Centralizing these checks using a standardized mechanism might reduce the incidence of these problems. Additionally, it might make it easier for future code analysis to catch similar problems.

If there is sufficient interest, I'd be happy to cut/paste this alternative, open it as a separate issue, and suggest a class name and some method signatures. Over the course of writing the original issue, I came to favor this alternative design.

Risks

Care must be taken to insure that the additional overloads do not create ambiguity with existing signatures if the "Alternative Designs" are not used.

[dotnet-issue-labeler]

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.

[gfoidl]

See #70515

[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

In many cases, I find that I check the validity of an argument with the sole intent of assigning it to a local variable or automatic property. With the current method signatures, this requires two lines, for example:

ArgumentNullException.ThrowIfNull(argument);
Argument = argument;

By adding a new overload, it would be possible to reduce clutter and perform both of these operations on a single line, for example:

Argument = ArgumentNullException.ThrowIfNull(argument);

This same issue exists in similar helper methods in the ArgumentException class. The addition of similar methods to the ArgumentOutOfRangeException class might also be helpful.

Alternatively, a single static class that combines common argument-checking scenarios might be preferable. I initially came here to champion overloading the existing class methods. However, after I completed the text for this issue, I became more convinced that the "Alternative Designs" discussed later are in fact a better approach.

API Proposal

namespace System;
...
public class ArgumentNullException : SystemException
{
  ...
  /// <summary>Throws an <see cref="ArgumentNullException"/> if <paramref name="argument"/> is null.</summary>
  /// <param name="argument">The reference type argument to validate as non-null.</param>
  /// <param name="paramName">The name of the parameter with which <paramref name="argument"/> corresponds.</param>
  [return: NotNull]
  public static T ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null);
  ...
}

Note: This is an example of a single argument-checking scenario. Additional scenarios are discussed in the "Alternative Designs" section.

API Usage

public class MyClass
{
  public MyClass(string? argument)
  {
    Argument = ArgumentNullException.ThrowIfNull(argument);
  }

  public string Argument { get; }
}

Alternative Designs

A new more-general helper class with static methods might be a preferable alternative to avoid the risk of signature ambiguity. I suggest the class should strive to achieve the following goals:

1. Use the System namespace to promote widespread use.
2. Have a short intuitive name to reduce clutter.
3. Limit the contents expressly to static methods that perform argument-checking.
4. Ensure that each static method accepts the widest possible range of appropriate types, using generics (constrained where appropriate).
5. Ensure that each static method either returns the original non-null argument value or throws an exception.
6. Mark similar existing methods (in ArgumentNullException and ArgumentException) as obsolete to promote uniform adoption of a more standardized mechanism.

I suggest the following minimal argument-checking scenarios should be considered:

1. Check if the argument is null (throws ArgumentNullException).
2. Check if the (string) argument is null or empty (throws ArgumentNullException/ArgumentException).
3. Check if the (string) argument is null, empty, or white space (throws ArgumentNullException/ArgumentException).
4. Check if the (comparable) argument is greater than or equal to a minimum value (throws ArgumentOutOfRangeException/ArgumentNullException).
5. Check if the (comparable) argument is less than or equal to the maximum value (throws ArgumentOutOfRangeException/ArgumentNullException).
6. Check if the (comparable) argument is within the specified inclusive minimum/maximum range (throws ArgumentOutOfRangeException/ArgumentNullException).

For what it's worth, in my own code, I've settled on the class name Guard and method names that closely resemble the existing method names (e.g. Guard.ThrowIfNull).

Finally, it's worth noting that inconsistent parameter ordering for the constructors of ArgumentNullException, ArgumentException, And ArgumentOutOfRangeException sometimes contributes to incorrect error reporting. Centralizing these checks using a standardized mechanism might reduce the incidence of these problems. Additionally, it might make it easier for future code analysis to catch similar problems.

If there is sufficient interest, I'd be happy to cut/paste this alternative, open it as a separate issue, and suggest a class name and some method signatures. Over the course of writing the original issue, I came to favor this alternative design.

Risks

Care must be taken to insure that the additional overloads do not create ambiguity with existing signatures if the "Alternative Designs" are not used.

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

[dakersnar]

Thanks for the suggestion. Closing as a dup of the above.

[techfan101]

@dakersnar while the title is indeed similar, if you read the "Alternate Designs", it is not a duplicate.

Many common argument-checking scenarios are currently not well-supported or well-standardized.

I understand your time is limited and there are only so many issues you can address. However, given the prevalence of argument-checking within code, I'd urge you to give better standardization of this process more careful consideration.

Perhaps it's better addressed with a more generalized issue/title, but it's an issue that's likely to keep popping up.

So far, at least two .NET developers have noted this issue. I suspect more would be found with a more thorough search. Additionally, a simple search will find a number of NuGet packages out there that try to address this same shortcoming.

[dakersnar]

@techfan101 Thanks for the follow-up. I'll reopen the issue to continue the discussion.

If there is sufficient interest, I'd be happy to cut/paste this alternative, open it as a separate issue, and suggest a class name and some method signatures. Over the course of writing the original issue, I came to favor this alternative design.

I suspect this might be the best course of action, but I'll let others weigh in first.

[stephentoub]

Many common argument-checking scenarios are currently not well-supported or well-standardized.

The approach we've standardized on is to add the relevant Throw helper to the corresponding exception type:

• ArgumentNullException.ThrowIfNull
• ArgumentException.ThrowIfNullOrEmpty
• ArgumentOutOfRangeException.ThrowIfZero
• ArgumentOutOfRangeException.ThrowIfNegative
• ArgumentOutOfRangeException.ThrowIfNegativeOrZero
• ArgumentOutOfRangeException.ThrowIfLessThan
• ArgumentOutOfRangeException.ThrowIfLessThanOrEqual
• ArgumentOutOfRangeException.ThrowIfGreaterThan
• ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual
• ObjectDisposedException.ThrowIf

Some of the above shipped in previous releases of .NET. All of the above are currently in main. If there's a specific case that you believe is missing, please feel free to open an issue on it if there isn't already one. The only such case I see from your list above is a ThrowIfNullOrWhiteSpace, which is covered by #77749.

Thanks.

[techfan101]

@dakersnar thank you for reconsidering. given the additional information from @stephentoub, please feel free to close this issue.

@stephentoub thank you for commenting. It's nice to see that there are some pending additions. I'll have to look at their details.

It seems that perhaps (beyond ThrowIfNullOrWhiteSpace) the only missing method (in ArgumentOutOfRangeException) is one that checks BOTH lower and upper bounds. Though, here I suppose two separate method calls work.

While I do have remaining concerns, which I'll share below, I don't think they rise to the level of a new issue. I do think there are better approaches than those chosen. However, I realize this is (to some extent) a matter of opinion/style.

1. With this approach, in the case where a single argument check might throw one of multiple exceptions, it's unclear to which exception class the method should belong. For example, does ThrowIfNullOrEmpty truly belong in ArgumentNullException or ArgumentException. Here, ArgumentException makes the most sense (since ArgumentNullException is a derivation). That said, I can imagine future scenarios where the conflict might not be so easily resolved.

2. Like the previous poster, I would prefer that these methods return the original argument (if valid). While I understand the arguments to the contrary, I do believe the lack of this feature (while perhaps more performant) creates a lot of additional clutter. It's no big deal for one argument check, but in code where there are hundreds of them it adds up.

3. I'm not a fan of the verbosity of the names of these all-too-common calls...ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual is a mouthful :)

At any rate, thank you for your time and consideration.

[stephentoub]

one that checks BOTH lower and upper bounds

Yup. There was a long discussion about that in the API review:
https://www.youtube.com/watch?v=_pCaRrqBQdQ&t=0h28m18s

[techfan101]

@stephentoub I'll have to take a look. For what it's worth, I think you made the correct decision.

In my own interim class, I (mistakenly) preferred fewer methods with both min and max checks, but without inclusive/exclusive bound checking. I think your choice provides clearer intent and better flexibility.

To be honest, there haven't been that many cases where I check both min and max. But there have been a ton of times when inclusivity or exclusivity of a bound was important.

[dakersnar]

Thanks for the suggestions and discussion. Closing this as resolved.