代码之家 › 专栏 › 技术社区 › Oleg Sh

将异步方法分成两部分进行代码分析?

sonarqube async-await c#

Oleg Sh · 技术社区 · 5 年前

我有密码:

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

0 回复 | 直到 5 年前

Peter Duniho 5 年前

假设你想遵循代码分析的建议,我不会做第一个方法 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();
}

尽管如此,在我看来,没有一个强有力的理由来分裂这样的方法。声纳库贝法则, Parameter validation in "async"/"await" methods should be wrapped 我是不是太谨慎了。

编译器在 异步 方法,就像迭代器方法那样。对于迭代器方法,在单独的方法中进行参数验证是有价值的,因为否则,只有调用方尝试获取序列中的第一个元素(即当编译器生成 MoveNext() 方法被调用)。

但是为了 异步 方法中的所有代码 await 语句(包括任何参数验证)将在对该方法的初始调用时执行。

SonarQube规则似乎基于一个问题,即 Task 观察到,在 异步 方法将不被遵守。这是真的。但是 异步 方法是 等待 返回的 任务 ,它将在完成时立即观察异常,这当然是在生成异常时发生的,并且将同步发生(即不会生成线程)。

我承认这不难也不快。例如,可以启动一些 异步 打电话,然后使用例如 Task.WhenAll() 观察他们的完成。如果没有立即的参数验证,在意识到其中一个调用无效之前,您将停止启动所有任务。这确实违反了“快速失败”的一般原则(这就是SonarQube规则的含义)。

但是,另一方面,参数验证失败几乎总是由于用户代码不正确造成的。也就是说,它们不会因为数据输入问题而发生,而是因为代码写得不正确。”在这种情况下,“快速失败”是一种奢侈;更重要的是,对我来说,代码是以一种自然的、易于遵循的方式编写的,我认为把所有东西放在一个方法中可以更好地实现这个目标。

所以在这种情况下,sonarqube给出的建议没有必要遵循。你可以离开 异步 方法作为一个单独的方法,您最初使用它的方式不会损害代码。甚至比迭代器方法场景(有相似的参数pro和con)更重要的是,imho也有同样多的理由将验证留在 异步 方法,以便将其移除到包装器方法。

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

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

也就是说,不要实施 异步 完全。你的代码不需要 异步 因为只有一个 等待 它发生在方法的最后。因为代码实际上不需要返回控件,所以实际上不需要 异步 . 只需执行所有需要执行的同步操作(包括参数验证),然后返回 任务 否则你早就等着了。

而且,我还要注意,这种方法同时解决了代码分析警告, 和