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 回答