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

问题描述

我有代码:

 公共异步任务DeleteColorSchemeAsync(ColorScheme colorScheme){如果(colorScheme == null)抛出新的ArgumentNullException(nameof(colorScheme));如果(colorScheme.IsDefault)抛出新的SettingIsDefaultException();_dbContext.ColorSchemes.Remove(colorScheme);等待_dbContext.SaveChangesAsync();} 

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

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

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

 公共异步任务DeleteColorSchemeAsync(ColorScheme colorScheme){如果(colorScheme == null)抛出新的ArgumentNullException(nameof(colorScheme));如果(colorScheme.IsDefault)抛出新的SettingIsDefaultException();等待DeleteColorSchemeInternalAsync(colorScheme);}私有异步任务DeleteColorSchemeInternalAsync(ColorScheme colorScheme){_dbContext.ColorSchemes.Remove(colorScheme);等待_dbContext.SaveChangesAsync();} 

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

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

解决方案

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

 公共任务DeleteColorSchemeAsync(ColorScheme colorScheme){如果(colorScheme == null)抛出新的ArgumentNullException(nameof(colorScheme));如果(colorScheme.IsDefault)抛出新的SettingIsDefaultException();返回DeleteColorSchemeInternalAsync(colorScheme);}私有异步任务DeleteColorSchemeInternalAsync(ColorScheme colorScheme){_dbContext.ColorSchemes.Remove(colorScheme);等待_dbContext.SaveChangesAsync();} 

所有这些，在我看来，并没有足够的理由来拆分这种方法.SonarQube的规则应该包装异步"/等待"方法中的参数验证是恕我直言，过于谨慎.

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

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

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

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

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

因此，在这种情况下，不必遵循SonarQube的建议.您可以将 async 方法保留为单一方法，保持原来的方式，而不会损害代码.甚至比迭代器方法方案(具有相似的正反参数)更是如此，恕我直言，将验证保留在 async 方法中的理由与将其删除到包装方法中的原因一样多./p>

但是，如果您确实选择遵循SonarQube的建议，那么我上面提供的示例将是恕我直言的一种比您拥有的方法更好的方法(并且的确，它更符合SonarQube文档中的详细建议).

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

 公共任务DeleteColorSchemeAsync(ColorScheme colorScheme){如果(colorScheme == null)抛出新的ArgumentNullException(nameof(colorScheme));如果(colorScheme.IsDefault)抛出新的SettingIsDefaultException();_dbContext.ColorSchemes.Remove(colorScheme);返回_dbContext.SaveChangesAsync();} 

即完全不要实现 async .您的代码不需要 async ，因为只有一个 await ，它发生在方法的最后.由于您的代码实际上并不需要将控制返回给它，因此实际上并不需要使它成为 async .只需完成您需要做的所有同步工作(包括参数验证)，然后返回原本要等待的 Task .

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

I have code:

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

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?

used code tool analyzator: sonarqube

解决方案

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

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.

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).

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.

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).

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).

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.

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.

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

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屋！