将异步方法分为两种进行代码分析？

Question

我有代码：

public async Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    _dbContext.ColorSchemes.Remove(colorScheme);
    await _dbContext.SaveChangesAsync();
}

一个代码分析器建议我将此方法分为两种方法：

将此方法分为两个部分，一个处理参数检查，另一个处理异步代码

我以下列方式拆分此代码时是否正确？

public async Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    await DeleteColorSchemeInternalAsync(colorScheme);
}

private async Task DeleteColorSchemeInternalAsync(ColorScheme colorScheme)
{
    _dbContext.ColorSchemes.Remove(colorScheme);
    await _dbContext.SaveChangesAsync();
}

编译器有什么不同？它看到了两种异步方法，与我的第一种方法有什么不同？

二手编码工具分析仪：sonarqube

Answer 1

当我按以下方式拆分此代码时，我是否正确？

不，正确的分割方式是这样的：

public Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    return DeleteColorSchemeInternalAsync(colorScheme);
}

private async Task DeleteColorSchemeInternalAsync(ColorScheme colorScheme)
{
    _dbContext.ColorSchemes.Remove(colorScheme);
    await _dbContext.SaveChangesAsync();
}

（注意，本例中没有 输入方法）。async

或者像这样，使用较新的本地函数：

public Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    return DeleteColorSchemeAsync();

    async Task DeleteColorSchemeAsync()
    {
        _dbContext.ColorSchemes.Remove(colorScheme);
        await _dbContext.SaveChangesAsync();
    }
}

存在此规则的原因是为了确保尽快抛出使用异常。如果您不拆分逻辑并将验证保留在方法内async，则仅当有人等待您返回的任务时才会引发异常，这可能不会立即发生，具体取决于使用情况。

一种非常常见的流程是，当您想要同时触发多个任务并等待它们完成时，提前抛出会很有好处。由于在此流程中，您的await操作是在任务触发后发生的，因此您将得到一个可能与调用的实际点相距甚远的异常，从而使调试变得不必要的困难。

接受的答案还建议在您只有一个异步操作要做（即结果值）的情况下直接返回任务，但是，这在调试代码时会带来严重问题，因为您的方法从堆栈跟踪中省略，使得导航变得更加困难在整个流程中。这是一个更深入讨论此问题的视频： https://youtu.be/Q2zDatDVnO0 ?t=327

仅对于极其简单的“中继”类型的方法才应执行不等待任务而直接返回任务的操作，其中父方法中的相关逻辑很少。

我建议始终遵守规则。

Answer 2

假设您要遵循代码分析建议，那么我不会采用第一种方法async。相反，它只能执行参数验证，然后返回调用第二个的结果：

public Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    return DeleteColorSchemeInternalAsync(colorScheme);
}

private async Task DeleteColorSchemeInternalAsync(ColorScheme colorScheme)
{
    _dbContext.ColorSchemes.Remove(colorScheme);
    await _dbContext.SaveChangesAsync();
}

综上所述，我认为没有足够的理由将这种方法分开。SonarQube的规则，即“异步” /“等待”方法中的参数验证应被包装，恕我直言过于谨慎。

编译器对async方法使用与对迭代器方法相同的转换。对于迭代器方法，在单独的方法中进行参数验证很有价值，因为否则，除非调用者尝试获取序列中的第一个元素（即，MoveNext()调用编译器生成的方法时），否则它不会完成。

但是对于async方法，方法中直到第一个await语句的所有代码，包括任何参数验证，都将在对该方法的初始调用时执行。

SonarQube规则似乎是基于这样一个担忧，即直到Task观察到，该async方法中生成的任何异常都不会被观察到。没错 但是方法的典型调用顺序async是await返回的Task，它会在完成后立即观察到异常，这当然会在生成异常时发生，并且会同步发生（即不会产生线程）。

我承认这不是一成不变的。例如，一个人可能会发起一定数量的async呼叫，然后使用Task.WhenAll()来观察其完成情况。如果没有立即进行参数验证，您将在意识到其中一个调用无效之前结束所有任务的启动。这确实违反了“快速失败”的一般原则（这是SonarQube规则所针对的原则）。

但是，另一方面，参数验证失败几乎总是归因于用户代码不正确。也就是说，它们不是因为数据输入问题而发生，而是因为代码编写不正确。在这种情况下，“快速失败”有点奢侈。无论如何，对我而言，更重要的是，以自然，易于遵循的方式编写代码，而且我认为将所有内容都放在一种方法中可以更好地实现该目标。

因此，在这种情况下，无需遵循SonarQube的建议。您可以将async方法保留为原来的单一方法，而不会破坏代码。与迭代器方法场景（具有相似的参数pro和con）相比，恕我直言，将IMHO保留在async方法中的理由与将其删除到包装方法的理由一样多。

但是，如果您确实选择遵循SonarQube的建议，那么我上面提供的示例将是恕我直言的比您拥有的方法更好的方法（的确，它更符合SonarQube文档的详细建议）。

我会注意到，实际上，有一种更简单的方式来表达代码：

public Task DeleteColorSchemeAsync(ColorScheme colorScheme)
{
    if (colorScheme == null)
        throw new ArgumentNullException(nameof(colorScheme));

    if (colorScheme.IsDefault)
        throw new SettingIsDefaultException();

    _dbContext.ColorSchemes.Remove(colorScheme);
    return _dbContext.SaveChangesAsync();
}

即根本不执行该实现async。您的代码不需要，async因为只有一个代码await，它发生在方法的最后。由于您的代码实际上并不需要将控制返回给它，因此实际上并不需要这样做async。只需完成您需要做的所有同步工作（包括参数验证），然后返回Task本来要等待的工作即可。

而且，我还将注意到，这种方法作为内置参数验证的单一方法，既解决了代码分析警告，又使实现保持简单。两全其美。:)