代码优化 - 删除这个总是评估为“真”的表达式

Posted 2023-03-16

【中文标题】代码优化 - 删除这个总是评估为“真”的表达式

【英文标题】：Code Optimisation - remove this expression which always evaluates to "true"

【发布时间】：2021-06-03 20:40:15

【问题描述】：

我正在尝试使用 SonarQube 优化代码

List<string> itemList = serviceObject.GetItems();

我尝试使用以下代码验证列表

if(itemList != null && itemList.Any()

     //Some operations

执行上述代码时，我收到 Sonarqube 错误 删除此表达式，该表达式的计算结果始终为“true” 所以我将代码重构为

if(itemList == null || !itemList.Any())
    return;
//Some Operations

执行上述代码时，我收到 Sonarqube 错误 删除此表达式，该表达式的计算结果始终为“false”

谁能告诉我这里出了什么问题？

【问题讨论】：

您是否尝试将= serviceObject.GetItems() 替换为= null，看看会发生什么。也许.GetItems() 方法中的某些东西确保它不为空？但这似乎是一个奇怪的警告。

能否贴出完整的代码。

@JakobBuskSørensen 除非启用可空引用类型，在这种情况下 itemList 不能为空。

您使用的是哪个 C# 版本？您是否通过在文件中添加#nullable enable 或在csproj 文件中添加<Nullable>enable</Nullable> 来启用可空引用类型？

serviceObject.GetItems() 返回什么类型？ .错误意味着 itemList 始终为空

【参考方案1】：

您可以将其缩短为

if (itemList?.Count >0)

...

或

if (itemList?.Any() ==true)

...

?. 是Null conditional operators 之一（另一个是?[]），简称Elvis operator，它允许您访问潜在的空变量而无需抛出。 Elvis 运算符之后的整个表达式的结果可以为 null，如果变量为 null，则返回 null。

这意味着itemList?.Count 返回一个Nullable<int>，而itemList?.Any() 返回一个Nullable<bool>。 Nullable<T> 定义了自身与其基类型 T 之间的关系运算符，但不能在没有显式转换的情况下用作 T。这就是需要(itemList?.Any() ==true) 的原因。

如果你使用Nullable Reference Types，itemList 不能为空，所以一个简单的比较就足够了：

if (itemList.Count >0)

...

如果通过在源文件中设置#nullable enable 或在csproj 文件中设置<Nullable>enable</Nullable> 来启用可空引用类型，编译器会确保所有返回引用类型的变量、字段和方法不是 em> null，迫使您要么解决问题，要么明确指定一个变量可以为空。

启用 NRT 后，此行：

List<string> itemList = serviceObject.GetItems();

如果GetItems 从未返回空值，则只会在没有警告的情况下编译。如果编译器对此表示怀疑，它会发出警告，建议您修复问题或明确声明 itemList 可以为空：

List<string>? itemList = serviceObject.GetItems();

【讨论】：

我也有同样的想法，但没有实现，因为我认为它只处理 True case 并且可能导致 SonarQube 代码异味。令我惊讶的是，它通过了 SonarQube。吸取教训而不是思考它会不会起作用，最好尝试一下并得出结论。非常感谢

【参考方案2】：

我相信这是由于空比较。

itemList != null

serviceObject.GetItems(); 很有可能通过 [NotNull] 注释保证不返回 null。因此，空检查是多余的。

How can I show that a method will never return null (Design by contract) in C#

【讨论】：

C# 中的NotNull 属性与或多或少被放弃的代码契约关系不大。当它无法确定结果或值的可空性时，它与 NRT 一起用于help the compiler。在这种情况下，对于 NRT，List<string> 表示变量不能为空。如果GetItems 返回List<string>?，编译器将生成警告

NotNull 是更清晰的东西，而不是 C# 的东西（不再），对吧？

@PanagiotisKanavos 是的，您对可空引用类型是正确的，我给出的解释具有误导性。

【参考方案3】：

if( itemList !=null ) //做某事...

【讨论】：

表达式检查 1 个或多个项目，而不仅仅是 null

@Panagiotis Kanavos 如果您知道，请发布正确的答案。

我在 20 分钟前做过，解释了发生了什么以及如何缩短表达式

@Panagiotis Kanavos 我有疑问，但没有人回答我...如果我找到了帮助...对我来说很容易

【参考方案4】：

List<string> itemList = new List<string>();
itemList = serviceObject.GetItems();
if(itemList!=null && itemList.Count() > 0 )

     //Some operations

Count 足以验证列表

List 永远不会为 null，但不会有任何元素。但是您必须将其实例化，以免引发异常。

【讨论】：

你不能调用任何方法（比如Count()，如果列表没有被实例化，即如果它是null）。因此，要使用Count() 或Any()，您必须首先确保列表不为空。

@JKC Jakob 说的是对的。我们不能指望服务不返回 null。最好检查 null

小心 Count();在某些情况下，这可能是性能灾难；见***.com/questions/305092/…

Jakob 的重点是“你怎么能绝对确定 serviceObject.GetItems() 没有返回 null？” - 如果是这样，你的代码就会爆炸。您的编辑并没有以任何方式解决这个问题，它只是让不正确的答案变得更糟

@JKC 这根本不是我要说的重点。以下是您的代码爆炸的方式：dotnetfiddle.net/JX5NzGdotnetfiddle.net/prqKJqdotnetfiddle.net/ydpvCo，即使在修复了您的代码无法编译的错误之后，因为 Count 是列表的属性，而不是方法（您的小提琴没有导入林克）。创建一个新列表只是用 GetItems 的返回值覆盖它是没有意义的，即使你做了itemList = service.GetItems()?itemList; 它也会仍然爆炸，因为新列表在 [0 处没有项目]