23

我已经在 Eclipse 中针对我的代码运行了PMD 插件,并且我收到了类似于下图所示代码的高优先级警告:

 if(singleRequest !=null){
   // do my work
 }else{
   // do my other work
 }

PMD 说`Avoid if (x != y) ..; else ..

错误的描述如下所示:

In an "if" expression with an "else" clause, avoid negation in
the test.  For example, rephrase:
if (x != y) diff(); else same();
as:
if (x == y) same(); else diff();
Most "if (x != y)" cases without an "else" are often return

但我仍然无法理解对我的代码的影响。如果有人可以举例指导我,我将不胜感激。

4

9 回答 9

33

许多 PMD 规则更像是风格意见而不是正确性警报。如果您不同意此规则或该规则与您项目的编码标准不匹配,您可以考虑抑制警告,甚至配置 PMD 以仅执行您喜欢的规则

于 2012-11-06T19:07:24.040 回答
13

PMD 是一种工具。PMD 基于启发式工作。有人决定采用这种启发式方法;带有 else 语句的否定条件不是“好风格”。

但是,在这种情况下,正如我在评论中所说,发布的代码就是将如何编写它。(特别是x != null,但不仅限于此构造。)

这是因为我不看条件(除了它可以被简化;例如移除双重否定,如 Jim Kin 所示),而是看分支或“流”的逻辑。

也就是说,我先放置正分支。在这种情况下,我认为

if (x != null) {
  doValid         // positive branch
} else {
  doFallback
}

在语义上等价于

if (isValid(x)) { // it looks like a "positive conditional" now
  doValid         // still positive branch
} else {
  doFallback
}

因此首先是正分支

当然,并非所有情况都有如此“清晰”的积极情绪,有些表达方式可能更容易以消极的方式表达。在这些情况下,我将“反转”分支 - 类似于 PMD 的建议 - 如果正分支/流被反转,通常会在块顶部添加注释说明操作。

可能影响使用的条件选择的另一个因素是“立即范围退出”分支,例如:

if (x == null) {
  // return, break, or
  throw new Exception("oops!");
} else {
  // But in this case, the else is silly
  // and should be removed for clarity (IMOHO) which,
  // if done, avoids the PMD warning entirely
} 

这就是 一贯的方式(除了一些偶尔的例外)编写我的代码:if (x != null) { .. }. 使用可用的工具;并让它们你工作。请参阅 Steven 的回答,了解如何在此处将 PMD 配置为更合适的“口味”。

于 2012-11-06T18:57:08.263 回答
9

这是一个可读性问题。考虑

if ( x != y ) 
{
}
else  // "if x doesn't not equal y"
{
}

对比

if ( x == y )
{
}
else  // "if x doesn't equal y"
{
}

后一个例子更容易识别。请注意,我认为使用底片没有任何问题......它可以更有意义,考虑一下

if ( x != null )...
于 2012-11-06T18:44:35.550 回答
4

我会避免使用否定案例的唯一原因是它是否会导致双重否定,这可能会令人困惑。

例如

if (!checkbox.disabled) {
    // checkbox is enabled
} 
else {
    // checkbox is disabled
}
于 2012-11-06T18:49:38.027 回答
3

谁读你的代码?你做。编译器会。或者也许是讲师的助手。一个无法区分 == 和 != 的同事?希望不是。

我只能认为否定词在复杂的表达中不好。(上下文是:至少对我来说。我知道我在调试时感到沮丧while(!expr && !expr2 || expr3) { }

ch=getch(); if (ch!='a') 是一种易于扩展的模式,
if (ch!='a' || ch!='b')它总是正确的,同时在语义上听起来是正确的。

从性能的角度来看,最好对概率进行排序。

if (more_probable) {
      ....
      unconditional_jump_to_end_of_block;
} else {
   ...
}

这种选择应该会带来更好的性能,因为在更可能的分支中没有错误预测的惩罚。

if (p && p->next)从性能的角度评估得出的结果很差。

于 2012-11-06T19:00:01.520 回答
1

您必须避免在 if 条件中出现“不等于”。这是因为当其他人查看您的代码时,该人很可能会忽略 != 并可能对您的程序逻辑得出错误的结论。

对于您的情况,您可能必须将 if 逻辑与 else 逻辑互换并将 != 更改为 ==

于 2012-11-06T18:46:25.673 回答
1

这是代码可读性与代码组织的平衡案例。该警告基本上表明人们阅读代码以导航否定的否定是令人困惑的。

我个人的经验法则是,无论您期望什么是“正常”情况,您都应该在 if 中进行测试。考虑:

 if (x != y) {
   // do work here...
 } else {
   throw new IllegalArgumentException();
 }

在这种情况下,我会说重要的工作正在这个x != y案例中完成,所以这就是你应该测试的。这是因为我喜欢组织代码,以便首先处理重要的工作,然后处理异常情况。

于 2012-11-06T18:47:02.163 回答
0

这是因为“良好的风格”表示如果可能的测试应该是“积极的”,所以:

if (singleRequest == null){
   // do my other work
} else {
   // do my work
}

更容易阅读,因为测试是“肯定的”(即“等于”而不是“不等于”),并且最终更好的可读性会导致更少的错误。

已编辑

尤其是像这样的测试:

if (!str.equals("foo")) {

你很容易错过!前面的,但如果你使测试呈阳性,它就会干净得多。

唯一一次你应该有一个否定测试是当没有else块时 - 然后一个否定测试是不可避免的,除非你有一个空true块,这本身被认为是一个风格问题。

于 2012-11-06T18:47:16.203 回答
0

不是真正的答案,但您可以通过提前返回或失败然后继续不缩进来最小化整体复杂性并提高可读性:

if (something == null) {
    throw new IllegalArgumentException("something must not be null");
}

// continue here
于 2017-08-22T07:59:18.297 回答