c# - 将异步方法一分为二进行代码分析？

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();
}

一个代码分析器建议我将此方法拆分为 2 个方法:

Split this method into two, one handling parameters check and the other handling the asynchronous code

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

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

假设您想遵循代码分析建议，我不会将第一个方法设为 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 的规则，Parameter validation in "async"/"await" methods should be wrapped恕我直言，是否过于谨慎。

编译器对 async 方法使用与迭代器方法相同的转换。使用迭代器方法，在单独的方法中进行参数验证是有值(value)的，因为否则直到调用者尝试获取序列中的第一个元素(即当编译器生成的 MoveNext() 方法被调用)。

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

SonarQube 规则似乎是基于这样的考虑，即在观察到 Task 之前，不会观察到 async 方法中生成的任何异常。这是真的。但是 async 方法的典型调用顺序是 await 返回的 Task，它会在完成时立即观察到异常，这当然会发生当异常产生时，将同步发生(即不会产生线程)。

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

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

所以在这种情况下，没有必要遵循 SonarQube 给出的建议。您可以将 async 方法保留为单个方法，就像您最初使用它的方式一样，而不会损害代码。甚至比迭代器方法场景(具有类似的赞成和反对论点)更重要的是，恕我直言，将验证留在 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。

而且，我还要指出，这种方法同时解决了代码分析警告，和保持实现简单，作为内置参数验证的单一方法。两全其美。 :)