Proposal: Adding a concise API for throwing exceptions

[jamesqo]

Background

Argument validation is done a lot in .NET code. So much, in fact, that people almost always find that writing throw new ArgumentNullException(...) et al. each time is too verbose, and end up writing their own helper classes to throw exceptions. Some random examples I picked from GitHub: Octokit.NET, xUnit, System.Collections.Immutable.

Proposal

We should have an API in the BCL for this so people don't have to keep writing their own helper classes. Here's what an initial prototype could look like (similar to the S.C.I class I linked to above):

public static class Requires
{
    public static void NotNull<T>(T value, string paramName); // no T : class constraint to allow nullables
    // parameter names are same as ArgumentNullException, to allow easy conversion
    // for people using named parameters
    public static void NotNull<T>(T value, string paramName, string message);

    public static void Range(bool condition, string paramName);
    public static void Range(bool condition, string paramName, string message);

    public static void Argument(bool condition, string message); // message comes first for ArgumentException
    public static void Argument(bool condition, string message, string paramName);
}

How it would be used:

T Single<T>(this IList<T> list)
{
    Requires.NotNull(list, nameof(list));
    Requires.Argument(list.Count == 1, "The input list needs to have only 1 item.");

    return list[0];
}

Additional Notes

In addition to providing a nice unified API, this may also lead to better performance / decreased memory consumption for .NET apps. Typically when people write their own helper APIs for this, they write it so that the throw is in the same clause as the check for the condition, i.e.

void NotNull<T>(T value, string name)
{
    if (value == null)
    {
        throw new ArgumentNullException(name);
    }
}

or, alternatively, they have a method that returns an exception and throw that from the caller:

ArgumentNullException ArgumentNull(string name)
{
    return new ArgumentNullException(name);
}

if (value == null) throw Error.ArgumentNull(nameof(value));

Code snippet 1 is not likely to be inlined because of the throw new ..., while ArgumentNull in code snippet 2 does get inlined, resulting in generated code bloat for a throw path that will very rarely get called.

If we wrote our own API for throwing exceptions, we could structure it so that the check gets inlined, but the throwing code doesn't:

void NotNull<T>(T value, string name)
{
    if (value == null)
    {
        ThrowArgumentNullException(name);
    }
}

void ThrowArgumentNullException(string name) => throw new ArgumentNullException(name);

Thoughts?

cc @svick @benaadams @jkotas @GSPP @mellinoe @KrzysztofCwalina

[prajaybasu]

I believe CodeContracts has a somewhat similar functionality ?

[danmoseley]

@terrajobst thoughts? is this api ready for review?

[danmoseley]

Code snippet 1 is not likely to be inlined because of the throw new ...

If I understand this change correctly, this is happily no longer true: https://github.com/dotnet/coreclr/commit/e76da565c106016d604b95f3bb7eadd24ceaf18a

[AlexGhiondea]

@jamesqo with the change @danmosemsft mentions, do you feel like adding such a 'throw helper' class is still something we need to consider?

[danmoseley]

There are some potential other advantages of throwing helpers (which is why I used them in other projects even though I was unaware they broke inlining then). Examples

1. ability to put a breakpoint without breaking on all such exceptions. It's easier to set a breakpoint than to go to the exceptions UI, also you can potentially set a conditional breakpoint.
2. ability to add tracing/logging in a single place for example in debug builds possibly with extra diagnostic information or checking of self consistency
3. ability to look up a localized string for the message
4. conditionally throw for example as a check for consistency or other check that is verbose to repeat inline or slow so you only want in debug builds

Having said that if the helpers are in the framework these points may be less relevant unless there is something like static callbacks you can set up.

Presumably the main motivation is to reduce code size and potentially help inlining, just as ThrowHelper still has value to corelib, it would have the same value to user code.

[jamesqo]

@AlexGhiondea @danmosemsft I am deciding to go in a different direction with this proposal. C# 7 introduces throw exprs where you can write _name = name ?? throw new ArgumentNullException(nameof(name)), so I feel like the current shape kind of conflicts with that since you can't use it with throw exprs.

@danmosemsft You mentioned in an earlier comment that methods with throw can now be inlined, so it might be better to have an API that returns exceptions instead:

public static class Error
{
    public static ArgumentNullException ArgumentNull(string name) =>
        new ArgumentNullException(name);

    public static ArgumentOutOfRangeException ArgumentOutOfRange(string name) =>
        new ArgumentOutOfRangeException(name);
}

_name = name ?? throw Error.ArgumentNull(nameof(name));

There would still be some code size benefit, albeit less, because the exception allocation would not happen inline in the caller.

[AlexGhiondea]

@jamesqo I like this approach!

I would rename the class to Exception or ExceptionFactory since we are basically building an exception factory.

And I am sure we can somehow auto-generate this for all exception types in the framework.

[jkotas]

_name = name ?? throw Error.ArgumentNull(nameof(name));

The outlining codesize optimization like this would be best done by the compilation toolchain ... works for billions lines of existing code, and you do not need to train million developers to write different code to take advantage of it.

[jamesqo]

@jkotas 👍 . I am deciding to retarget this again as a convenience-oriented API rather than a perf-oriented one, and also add bulk validation for common patterns, but I feel like I've flip flopped too many times in one proposal. I'll close this and open another one with the new shape I have in mind