CollapsibleIfStatements

Question

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を非アクティブにすることなく）。

Answer 1

一つの可能​​性は、ネストされたif文を繰り返しobj.getSomething()を考慮して、崩壊することです。

代わりに、ListをSetに置き換え、明示的にのチェックを行わないでください。 Setを使用すると、計算の複雑さが増します。

Answer 2

私は、これは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); 
   

Answer 3

それは折りたたみ可能です：

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

私見では、このチェックは時々役に立つことができますが、一部では無視されなければなりませんその他の場合。鍵は、できるだけコードを読みやすくすることです。コードが折りたたまれたif文より読みやすいと思われる場合は、@SuppressWarnings("PMD.CollapsibleIfStatements")注釈を追加してください。私の目には

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

が、結果は非常に読みやすい、そしてもはやPMDの警告をトリガするべきではない：