Mog0
Mog0

Reputation: 2129

Why is wrapping parameter validation in a non-async method desirable?

I've had this warning pop up in SonarQube but I don't understand why wrapping the validation is desirable.

I've read the following questions but neither appears to explain clearly WHY it is better?

Parameter validation in "async"/"await" methods should be wrapped

Validate parameters in async method

In the example

public async Task DoSomethingAsync(string param){
    if(string.IsNullOrEmpty(param)
    {
        throw new ArgumentException("Param is blank");
    }

    await DoSomethingElseAsync(param);
}

Why would this be executed any differently to

public Task DoSomethingAsync(string param){
    if(string.IsNullOrEmpty(param)
    {
        throw new ArgumentException("Param is blank");
    }

    return doSomethingElseAsync(param);
}

Given that both implementations would need to be awaited by the caller, would there actually be any guarantee that the validation in the 2nd implementation would be executed immediately rather than be delayed like any other async method?

Upvotes: 1

Views: 463

Answers (2)

Theodor Zoulias
Theodor Zoulias

Reputation: 43845

Assuming that the caller will invoke and await the asynchronous method sequentially, there is no difference.

await DoSomethingAsync("Chess");
await DoSomethingAsync("Stratego");
await DoSomethingAsync(null);

But how can you be sure about that? The caller might just as well separate the phase of creating the tasks, and the phase of awaiting them.

var tasks = new List<Task>();
tasks.Add(DoSomethingAsync("Chess"));
tasks.Add(DoSomethingAsync("Stratego"));
tasks.Add(DoSomethingAsync(null));
await Task.WhenAll(tasks);

In this case the two different DoSomethingAsync implementations will result to different behaviors in the caller's code.

One more difference is with the XML documentation of the method, assuming that you want to be consistent with Microsoft's guidelines. The first implementation requires no <exception> element, because no exception can be thrown synchronously:

/// <summary>Does something asynchronously.</summary>
/// <param name="param">The parameter.</param>
/// <returns>A <see cref="Task"/> that will complete when something is done.</returns>
public async Task DoSomethingAsync(string param)
{
    // ...
}

The second implementation does require an <exception> element, because an ArgumentException can be thrown synchronously:

/// <summary>Does something asynchronously.</summary>
/// <param name="param">The parameter.</param>
/// <returns>A <see cref="Task"/> that will complete when something is done.</returns>
/// <exception cref="ArgumentException"><paramref name="param"/> is blank.</exception>
public Task DoSomethingAsync(string param)
{
    // ...
}

Upvotes: 1

Paulo Morgado
Paulo Morgado

Reputation: 14856

An asynchronous method is converted by the compiler to a state machine.

When the method is invoked, the state machine is instantiated. And only then, the code before the first await is invoked.

If you have this method:

private static async Task<int> GetValueAsync(object o)
{
    if (o is null) throw new ArgumentNullException(nameof(o));
    await Task.Delay(1);
    return 0;
}

The generated code will be:

[StructLayout(LayoutKind.Auto)]
[CompilerGenerated]
private struct <GetValueAsync>d__0 : IAsyncStateMachine
{
    public int <>1__state;

    public AsyncTaskMethodBuilder<int> <>t__builder;

    public object o;

    private TaskAwaiter <>u__1;

    private void MoveNext()
    {
        int num = <>1__state;
        int result;
        try
        {
            TaskAwaiter awaiter;
            if (num != 0)
            {
                if (o == null)
                {
                    throw new ArgumentNullException("o");
                }
                awaiter = Task.Delay(1).GetAwaiter();
                if (!awaiter.IsCompleted)
                {
                    num = (<>1__state = 0);
                    <>u__1 = awaiter;
                    <>t__builder.AwaitUnsafeOnCompleted(ref awaiter, ref this);
                    return;
                }
            }
            else
            {
                awaiter = <>u__1;
                <>u__1 = default(TaskAwaiter);
                num = (<>1__state = -1);
            }
            awaiter.GetResult();
            result = 0;
        }
        catch (Exception exception)
        {
            <>1__state = -2;
            <>t__builder.SetException(exception);
            return;
        }
        <>1__state = -2;
        <>t__builder.SetResult(result);
    }

    void IAsyncStateMachine.MoveNext()
    {
        //ILSpy generated this explicit interface implementation from .override directive in MoveNext
        this.MoveNext();
    }

    [DebuggerHidden]
    private void SetStateMachine(IAsyncStateMachine stateMachine)
    {
        <>t__builder.SetStateMachine(stateMachine);
    }

    void IAsyncStateMachine.SetStateMachine(IAsyncStateMachine stateMachine)
    {
        //ILSpy generated this explicit interface implementation from .override directive in SetStateMachine
        this.SetStateMachine(stateMachine);
    }
}

[AsyncStateMachine(typeof(<GetValueAsync>d__0))]
private static Task<int> GetValueAsync(object o)
{
    <GetValueAsync>d__0 stateMachine = default(<GetValueAsync>d__0);
    stateMachine.<>t__builder = AsyncTaskMethodBuilder<int>.Create();
    stateMachine.o = o;
    stateMachine.<>1__state = -1;
    stateMachine.<>t__builder.Start(ref stateMachine);
    return stateMachine.<>t__builder.Task;
}

If you validate the parameters in a non-async method:

private static Task<int> GetValueAsync(object o)
{
    if (o is null) throw new ArgumentNullException(nameof(o));
    return GetValueAsyncImpl(o);

    static async Task<int> GetValueAsyncImpl(object o)
    {
        await Task.Delay(1);
        return 0;
    }
}

The generated code will be:

[StructLayout(LayoutKind.Auto)]
[CompilerGenerated]
private struct <<GetValueAsync>g__GetValueAsyncImpl|0_0>d : IAsyncStateMachine
{
    public int <>1__state;

    public AsyncTaskMethodBuilder<int> <>t__builder;

    private TaskAwaiter <>u__1;

    private void MoveNext()
    {
        int num = <>1__state;
        int result;
        try
        {
            TaskAwaiter awaiter;
            if (num != 0)
            {
                awaiter = Task.Delay(1).GetAwaiter();
                if (!awaiter.IsCompleted)
                {
                    num = (<>1__state = 0);
                    <>u__1 = awaiter;
                    <>t__builder.AwaitUnsafeOnCompleted(ref awaiter, ref this);
                    return;
                }
            }
            else
            {
                awaiter = <>u__1;
                <>u__1 = default(TaskAwaiter);
                num = (<>1__state = -1);
            }
            awaiter.GetResult();
            result = 0;
        }
        catch (Exception exception)
        {
            <>1__state = -2;
            <>t__builder.SetException(exception);
            return;
        }
        <>1__state = -2;
        <>t__builder.SetResult(result);
    }

    void IAsyncStateMachine.MoveNext()
    {
        //ILSpy generated this explicit interface implementation from .override directive in MoveNext
        this.MoveNext();
    }

    [DebuggerHidden]
    private void SetStateMachine(IAsyncStateMachine stateMachine)
    {
        <>t__builder.SetStateMachine(stateMachine);
    }

    void IAsyncStateMachine.SetStateMachine(IAsyncStateMachine stateMachine)
    {
        //ILSpy generated this explicit interface implementation from .override directive in SetStateMachine
        this.SetStateMachine(stateMachine);
    }
}

private static Task<int> GetValueAsync(object o)
{
    if (o == null)
    {
        throw new ArgumentNullException("o");
    }
    return <GetValueAsync>g__GetValueAsyncImpl|0_0(o);
}

[AsyncStateMachine(typeof(<<GetValueAsync>g__GetValueAsyncImpl|0_0>d))]
[CompilerGenerated]
internal static Task<int> <GetValueAsync>g__GetValueAsyncImpl|0_0(object o)
{
    <<GetValueAsync>g__GetValueAsyncImpl|0_0>d stateMachine = default(<<GetValueAsync>g__GetValueAsyncImpl|0_0>d);
    stateMachine.<>t__builder = AsyncTaskMethodBuilder<int>.Create();
    stateMachine.<>1__state = -1;
    stateMachine.<>t__builder.Start(ref stateMachine);
    return stateMachine.<>t__builder.Task;
}

Which does not instantiate the state machine if there are invalide parameter values.

The same happens for iterator methods.

Upvotes: 0

Related Questions