将异步方法一分为二进行代码分析? [英] Split async method into two for code analysis?

问题描述

我有代码:

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 种方法:

One code analyzer recommends me to split this method to 2 methods:

这个方法一分为二，一个处理参数检查，一个处理异步代码

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

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

Am I correct, when I split this code in the following way?

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

编译器有什么不同?它看到两个异步方法，与我的第一个变体有什么不同?

What is different for the compiler? It sees two async methods, what is different from my first variant?

使用的代码工具分析器:sonarqube

used code tool analyzator: sonarqube

推荐答案

假设您想遵循代码分析建议，我不会将第一个方法 async.相反，它可以只做参数验证，然后返回调用第二个的结果:

Assuming you wanted to follow the code analysis advice, I would not make the first method async. Instead, it can just do the parameter validation, and then return the result of calling the second:

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"/await"方法中的参数验证恕我直言，过于谨慎.

All that said, in my opinion there's not really a strong justification to split the method like that. The SonarQube's rule, Parameter validation in "async"/"await" methods should be wrapped is IMHO overly cautious.

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

The compiler uses the same kind of transformation on async methods as it does for iterator methods. With an iterator method, there is value in doing parameter validation in a separate method, because otherwise it won't be done until the caller tries to get the first element in the sequence (i.e. when the compiler-generated MoveNext() method is called).

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

But for async methods, all of the code in the method up to the first await statement, including any parameter validation, will be executed on the initial call to the method.

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

The SonarQube rule appears to be based on a concern that until the Task is observed, any exception generated in the async method won't be observed. Which is true. But the typical calling sequence of an async method is to await the returned Task, which will observe the exception immediately on completion, which of course occurs when the exception is generated, and will happen synchronously (i.e. the thread won't be yielded).

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

I admit that this is not hard-and-fast. For example, one might initiate some number of async calls, and then use e.g. Task.WhenAll() to observe their completions. Without immediate parameter validation, you would wind up starting all of the tasks before realizing that one of the calls was invalid. And this does violate the general principle of "fail-fast" (which is what the SonarQube rule is about).

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

But, on the other hand, parameter validation failures are almost always due to user code incorrectness. I.e. they don't happen because of data input problems, but rather because the code was written incorrectly. "Fail-fast" is a bit of a luxury in that context; what's more important, to me anyway, is that the code be written in a natural, easy-to-follow way, and I'd argue that keeping everything in one method achieves that goal better.

所以在这种情况下，SonarQube 给出的建议没有必要遵循.您可以将 async 方法保留为单个方法，就像您最初使用它的方式一样，不会损害代码.甚至比迭代器方法场景(具有相似的参数 pro 和 con)更重要的是，恕我直言，将验证留在 async 方法中的理由与将其删除到包装器方法中的理由一样多.

So in this case, the advice being given by SonarQube isn't necessary to follow. You can just leave your async method as a single method, the way you had it originally without harming the code. Even more so than the iterator method scenario (which has similar arguments pro and con), there is IMHO just as much reason to leave the validation in the async method as to remove it to a wrapper method.

但是，如果您确实选择遵循 SonarQube 的建议，我在上面提供的示例是恕我直言，比您现有的方法更好(实际上，更符合 SonarQube 文档中的详细建议).

But if you do choose to follow SonarQube's advice, the example I provided above is IMHO a better approach than what you have (and indeed, is more in line with the detailed advice on SonarQube's documentation).

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

I will note that in fact, there's an even simpler way to express the code:

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.

I.e. don't make the implementation async at all. Your code doesn't need async because there's only one await and it occurs at the very end of the method. Since your code doesn't actually need control returned to it, there's not actually any need to make it async. Just do all the synchronous stuff you need to do (including parameter validation), and then return the Task you'd otherwise have awaited.

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

And, I'll note as well, this approach addresses both the code analysis warning, and keeps the implementation simple, as a single method with parameter validation built-in. Best of both worlds. :)

这篇关于将异步方法一分为二进行代码分析?的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！