2014-02-24 60 views
10

此代碼:FindBugs的被反對匿名內部類

Set<Map.Entry<String, SSGSession>> theSet = new TreeSet<Map.Entry<String, SSGSession>>(new Comparator<Map.Entry<String, SSGSession>>() { 

     @Override 
     public int compare(final Map.Entry<String, SSGSession> e1, final Map.Entry<String, SSGSession> e2) { 
      return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime()); 
     } 
    })); 

觸發聲納違反,跳閘FindBugs的規則「SIC_INNER_SHOULD_BE_STATIC_ANON」,其具有的描述:

該類是一個內部類,但不會將其嵌入的參考 用於創建它的對象。此參考使類的實例更大,並且可能會將創建者對象 的引用保留更長的時間。如果可能的話,應該將課程設爲 爲靜態內部類。由於匿名內部類不能被標記爲靜態的 ,因此這將需要重構內部類 ,以便它是一個命名的內部類。

真的嗎?這不是很挑剔嗎?我是否應該真正重構匿名內部類中的單行方法以節省額外引用的成本?在這種情況下,它不可能將參考持續時間超過必要的時間。

我不介意這樣做,因爲我們強制執行的編碼標準是「零聲納違規」,但我強烈地試圖在這裏爭辯//NOSONAR的情況,因爲imho將一行方法提取到靜態內部使得代碼稍微難以溝通。

java純粹主義者認爲什麼?

+1

由於該類是無狀態的,因此可以更好地將比較器聲明爲「靜態final」,對所有調用使用相同的實例,而不是每次都創建一個新的實例。 –

+1

靜態代碼分析工具的目的不是挑剔嗎?對此抱怨有明確的技術理由。如果您不希望某些項目使用該規則,請禁用該規則。 – hyde

+2

...繼續下去,即使在某些情況下某些規則的應用似乎沒有必要,但我會採取立場,即禁用該規則的規則需要一個* strong *原因,但這不夠強。禁用整個規則,或遵循它。 – hyde

回答

10

將評論轉換爲答案,首先我可以說服,將這個匿名內部類作爲匿名內部類是有道理的,即使有明顯的技術原因對此不屑一顧。

不過,我會說:請按照您設定的規則。規則創建一致性,並且當所有代碼都以相同的方式編寫時,代碼庫更容易理解爲整體。如果某些規則不好,請將其禁用。

當出現異常時,還需要解釋爲什麼會有異常情況:爲閱讀代碼的人帶來額外的精神負擔,在代碼審查中討論額外的項目等。只有在個案中禁用規則,如果您可以爭論它是某種程度上的例外的情況。

此外,我不確定這樣做,因爲即使增加了更多樣板文件(如果下面的代碼不是100%正確的代碼,我的Java有點生疏,感覺自由與其他靜態類

Set<Map.Entry<String, SSGSession>> theSet 
    = new TreeSet<Map.Entry<String, SSGSession>>(new SSGSessionStartTimeComparator()); 
文件中

然後在其他地方,一起:建議編輯)

static class SSGSessionStartTimeComparator extends Comparator<Map.Entry<String, SSGSession>>() { 
    @Override 
    public int compare(final Map.Entry<String, SSGSession> e1, final Map.Entry<String, SSGSession> e2) { 
     return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime()); 
    } 
} 
+1

你可以在'SSGSessionStartTimeComparator'的聲明中添加'implements Serializable',以便也遵循SE_COMPARATOR_SHOULD_BE_SERIALIZABLE規則(不在這個問題的範圍內,但通常是個好主意)。 –

0

有三種解決方案,在這裏,這最好是在你的控制:

  • 擴展Java語法:

    ... theSet = new static Comparator ... 
    
  • 聲明和使用一個靜態類中所述。

  • 忽略這一個實例警告:

    @SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON") 
    ... your method ... 
    

我更喜歡第一個,但這是一個漫長的時間來如果有的話。因此,我會在最後忽略項目範圍內的規則。選擇一條規則應該有一點點痛苦來覆蓋它。否則這只是一個約定或建議。

4

爲了完整起見,我想爲已經提供的優秀答案添加另一個變體。定義一個常數Comparator,並使用:

private static final Comparator<Map.Entry<String, SSGSession>> BY_STARTTIME = 
     new Comparator<Map.Entry<String, SSGSession>>() { 
    @Override 
    public int compare(final Map.Entry<String, SSGSession> e1, 
      final Map.Entry<String, SSGSession> e2) { 
     return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime()); 
    } 
}; 

private void foo() { 
    Set<Map.Entry<String, SSGSession>> theSet = 
     new TreeSet<Map.Entry<String, SSGSession>>(BY_STARTTIME); 
} 

這將爲你省去額外類爲hyde's answer。否則,海德的回答會更好,因爲它允許您聲明Comparator是可序列化的(因爲它沒有狀態)。如果Comparator不可序列化,則您的TreeSet也不可序列化。

0

請注意:匿名內部類是泄漏內存的好方法,特別是在JEE bean中使用時。簡單的事情如下: new HashMap<>() {{ put"("a","b"); }};

用@ javax.ejb註釋的bean可能導致多個instanced保持活動狀態,因爲Map保持對bean的引用。