runtime: [API Proposal]: ArgumentException.ThrowIfNullOrEmpty(string)

Background and motivation

Inspired by ArgumentNullException.ThrowIfNull(object). string.IsNullOrEmpty(text) is a frequent check especially in web/Console project. At first I thought InvalidDataException would be the best name but it is in System.IO namespace, so I think the closest should be ArgumentException so user doesn’t need to import that namespace.

Now:

if (string.IsNullOrEmpty(input)) { throw new ArgumentException($"{nameof(input)} must contain value."); }

If the API is approved:

ArgumentException.ThrowIfNullOrEmpty(input);

While we are at it, personally I rarely use it but maybe ThrowIfNullOrWhiteSpace may be useful for others as well?

API Proposal

namespace System
{
    public class ArgumentException : SystemException
    {
        public static void ThrowIfNullOrEmpty([NotNull] string? argument, [CallerArgumentExpression("argument")] string? paramName = null)
    }
}

API Usage

ArgumentException.ThrowIfNullOrEmpty(input);

Alternative Designs

No response

Risks

No response

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 4
Comments: 24 (19 by maintainers)

Most upvoted comments

The argument to the existing ThrowIfNull is declared as:

[NotNull] object? argument

which uses ? to avoid warnings when being used with something that may be null, but then [NotNull] to say “upon successful return from this method, it’s guaranteed argument isn’t null.”

stephentoub on Dec 10, 2021

Guys check suggestion about ICollection above please 😃

hrrrrustic on Jan 4, 2022

Maybe it would also make sense to add overload with IReadOnlyCollection<T> or simply ICollection<T>?

public static void ThrowIfNullOrEmpty<T>([NotNull] I/*ReadOnly*/Collection<T> argument, [CallerArgumentExpression("argument")] string? paramName = null);

Usages: https://github.com/dotnet/runtime/blob/895c99cc502913fd7393b95af7cae5b9e6fbdea3/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLog.cs#L264-L265

https://github.com/dotnet/runtime/blob/895c99cc502913fd7393b95af7cae5b9e6fbdea3/src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Certificate.cs#L84-L85

Didn’t try to find splitted if-checks

Makes sense to me. It would collapse stuff like

https://github.com/dotnet/runtime/blob/dfbae37e91c4744822018dde10cbd414c661c0b8/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs#L44-L52

from 10 lines to 1.

In that example, it has a custom message, ie “Empty name is not legal.”, and using this proposed API would change that to something generic. I think that’s totally fine. There are cases where perhaps the message adds slightly more value eg “Empty file name is not legal.” and perhaps those would not change to use this API, which is fine.

hrrrrustic on Dec 15, 2021

@datvm sounds good, if you are interested there are issues with the ‘easy’ label and perhaps there’s one you might be interested to start with.

Anyone else interested in offering a PR here?

Doesn’t seem like a big deal. If it’s nothing urgent, I’m happy to do it.

deeprobin on Jan 12, 2022

@datvm sounds good, if you are interested there are issues with the ‘easy’ label and perhaps there’s one you might be interested to start with.

Anyone else interested in offering a PR here?

danmoseley on Jan 11, 2022

To conform to the string overloads, it would also make sense to create an overload for IsNullOrWhiteSpace.

namespace System
{
    public partial class ArgumentException
    {
        public static void ThrowIfNullOrEmpty([NotNull] string? argument,
                                              [CallerArgumentExpression("argument")] string? paramName = null);
        public static void ThrowIfNullOrWhiteSpace([NotNull] string? argument,
                                              [CallerArgumentExpression("argument")] string? paramName = null);
    }
}

deeprobin on Jan 10, 2022

Makes sense to me. It would collapse stuff like https://github.com/dotnet/runtime/blob/dfbae37e91c4744822018dde10cbd414c661c0b8/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs#L44-L52

from 10 lines to 1.

In that example, it has a custom message, ie “Empty name is not legal.”, and using this proposed API would change that to something generic. I think that’s totally fine. There are cases where perhaps the message adds slightly more value eg “Empty file name is not legal.” and perhaps those would not change to use this API, which is fine.

danmoseley on Dec 14, 2021