5

我最近使用 PMD（嵌入在 hudson 中）偶然发现了以下警告，我的代码似乎受到CollapsibleIfStatements的影响，我并不完全理解。代码看起来像这样

// list to be filled with unique Somethingness
List list = new ArrayList();

// fill list
for (SomeObject obj : getSomeObjects()) { // interating 
  if (!obj.getSomething().isEmpty()) { // check if "Something" is empty *
    if (!list.contains(obj.getSomething())) { // check if "Something" is already in my list **
      list.add(obj.getSomething()); // add "Something" to my list
    }
  }
}

在我看来，这段代码并不是更“可折叠”（否则下一个阅读代码的人会更加难以理解）。另一方面，我想解决这个警告（不停用 PMD ;）。

4

3 回答 3

7

一种可能性是排除重复的obj.getSomething()然后折叠嵌套if语句：

for (SomeObject obj : getSomeObjects()) {
  Something smth = obj.getSomething();
  if (!smth.isEmpty() && !list.contains(smth)) {
      list.add(smth);
  }
}

在我看来，结果非常易读，不应再触发 PMD 警告。

另一种方法是List用 a替换Set，并完全避免显式contains()检查。使用 aSet也会有更好的计算复杂度。

于 2012-02-23T10:27:36.757 回答
3

我认为这就是 PMD 想要的：

if (!obj.getSomething().isEmpty() && !list.contains(obj.getSomething())) {
  list.add(obj.getSomething());
}

以下是一些改进技巧（并防止 PMD 尖叫）：

  1. 提取变量：

    final Something sth = obj.getSomething();
if (!sth.isEmpty() && !list.contains(sth)) {
  list.add(sth);
}

  2. 提取方法Something（更多OOD）：

    class Something {

    public void addToListIfNotEmpty(List<Something> list) {
        if(isEmpty() && list.contains(this)) {
            list.add(this);
        }
    }

}

    并用简单的替换整个条件：

    obj.getSomething().addToListIfNotEmpty(list);
于 2012-02-23T10:26:56.497 回答
3

它是可折叠的：

if (!obj.getSomething().isEmpty() && !list.contains(obj.getSomething())) {
    list.add(obj.getSomething());
}

恕我直言，此检查有时很有用，但在其他情况下应忽略。关键是让代码尽可能地可读。如果您觉得您的代码比折叠的 if 语句更具可读性，请添加@SuppressWarnings("PMD.CollapsibleIfStatements")注释。

于 2012-02-23T10:27:32.510 回答