1. 程式人生 > >FindBugs錯誤修改指南 【轉】

FindBugs錯誤修改指南 【轉】

早期 雙重檢查鎖 一點 sub 整理 there chan 調度 又是

FindBugs錯誤修改指南

1. EC_UNRELATED_TYPES
Bug: Call to equals() comparing different types Pattern id: EC_UNRELATED_TYPES, type: EC, category: CORRECTNESS
解釋:
兩個不同類型的對象調用equals方法,如果equals方法沒有被重寫,那麽調用object的==,永遠不會相等;如果equals方法被重寫,而且含有instanceof邏輯,那麽還是不會相等。
解決方法:
應該改為str.toString()
2. IM_BAD_CHECK_FOR_ODD
Bug: Check for oddness that won‘t work for negative numbers Pattern id: IM_BAD_CHECK_FOR_ODD, type: IM, category: STYLE
解釋:
如果row是負奇數,那麽row % 2 == -1,
解決方法:
考慮使用x & 1 == 1或者x % 2 != 0
3. NP_ALWAYS_NULL
Pattern: Null pointer dereference id: NP_ALWAYS_NULL, type: NP, category: CORRECTNESS
A null pointer is dereferenced here. This will lead to a NullPointerException when the code is executed.
4. RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
Bug: Redundant nullcheck of bean1, which is known to be non-null Pattern id: RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE, type: RCN, category: STYLE
This method contains a redundant check of a known non-null value against the constant null.
這種方法包含了一個稱為非空對空值的不斷重復檢查。
修改為:
5. SS_SHOULD_BE_STATIC
Bug: Unread field: ADDRESS_KEY; should this field be static? Pattern id: SS_SHOULD_BE_STATIC, type: SS, category: PERFORMANCE
This class contains an instance final field that is initialized to a compile-time static value. Consider making the field static.
解釋:
final成員變量表示常量,只能被賦值一次,賦值後值不再改變。
這個類包含的一個final變量初始化為編譯時靜態值。考慮變成靜態常量
解決方法:
增加static關鍵字
6. EQ_COMPARETO_USE_OBJECT_EQUALS
Bug: RsInterface defines compareTo(Object) and uses Object.equals() Pattern id: EQ_COMPARETO_USE_OBJECT_EQUALS, type: Eq, category: BAD_PRACTICE
解釋:
第一段代碼,沒有使用instanceof判斷就直接轉型,有拋出classcastexception異常的可能。
這個BUG主題是,遵守約定(x.compareTo(y)==0) == (x.equals(y)),強烈建議,但不嚴格要求。
在return 0的時候,調用equals方法返回true,因為在PriorityQueue.remove方法中,1.5使用的是compareTo方法,而1.6使用的是equals方法,保證環境升級的時候,受影響最小。
解決方法:
在return 0的時候,調用equals方法返回true
7. NM_METHOD_NAMING_CONVENTION
Bug: The method name MsmPlanDAOTest.TestViewMsmPlanList() doesn‘t start with a lower case letter Pattern id: NM_METHOD_NAMING_CONVENTION, type: Nm, category: BAD_PRACTICE
Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized.
解釋:
方法應該是動詞,與第一個字母小寫混合的情況下,與每個單詞的首字母大寫的內部。
解決方法:
方法名稱小寫就通過了。
8. HE_EQUALS_USE_HASHCODE
Bug: PerfmSingleGraphPanel$RSCategory defines equals and uses Object.hashCode() Pattern id: HE_EQUALS_USE_HASHCODE, type: HE, category: BAD_PRACTICE
解釋:
重載了equals方法,卻沒有重載hashCode方法,如果使用object自己的hashCode,我們可以從JDK源代碼可以看到object的hashCode方法是native的,它的值由虛擬機分配(某種情況下代表了在虛擬機中的地址或者唯一標識),每個對象都不一樣。所以這很可能違反“Equals相等,hashcode一定相等;hashcode相等,equals不一定相等。”除非你保證不運用到HashMap/HashTable等運用散列表查找值的數據結構中。否則,發生任何事情都是有可能的。
關於何時改寫hashcode,請參考:在重寫了對象的equals方法後,還需要重寫hashCode方法嗎?
關於編寫高質量的equals方法:
1.先使用==操作符檢查是否是同一個對象,==都相等,那麽邏輯相等肯定成立;
2.然後使用instanceof操作符檢查“參數是否為正確的類型”;
3.把參數轉換成正確的類型;
4.對於該類中的非基本類型變量,遞歸調用equals方法;
5.變量的比較順序可能會影響到equals方法的性能,應該最先比較最有可能不一致的變量,或者是開銷最低的變量。
當你編寫完成equals方法之後,應該問自己三個問題:它是否是對稱的、傳遞的、一致的?
解決方法:
除非你保證不運用到HashMap/HashTable等運用散列表查找值的數據結構中,請重寫hashcode方法。
9. NM_CONFUSING
Bug: Confusing to have methods xxx.SellerBrandServiceImpl.getAllGrantSellerBrandsByBrandId(long) and xxx.DefaultSellerBrandManager.getALLGrantSellerBrandsByBrandId(long) Pattern id: NM_CONFUSING, type: Nm, category: BAD_PRACTICE
The referenced methods have names that differ only by capitalization.
解釋:
同一個包兩個類中有一模一樣的兩個方法(包括參數)
解決方法:
最好可以修改為不一樣的方法名稱
10. MF_CLASS_MASKS_FIELD
Bug: Field PDHSubCardInstanceDialogCommand.m_instance masks field in superclass ViewNEProperity Pattern id: MF_CLASS_MASKS_FIELD, type: MF, category: CORRECTNESS
This class defines a field with the same name as a visible instance field in a superclass. This is confusing, and may indicate an error if methods update or access one of the fields when they wanted the other.
解釋:
這是什麽意思呢?想要字段也能夠具有多態性嗎?太迷惑了。
當你想要更新一個m_instance時,你要更新哪個?你用到它時,你知道哪個又被更新了?
解決方法:
要麽去掉其中一個字段,要麽重新命名。
11. NM_CLASS_NAMING_CONVENTION
Bug: The class name crossConnectIndexCollecter doesn‘t start with an upper case letter
解釋: Pattern id: NM_CLASS_NAMING_CONVENTION, type: Nm, category: BAD_PRACTICE
看到這樣的命名方式,我第一個反映就是有點暈車!
解決方法:
類名第一個字符請大寫。
12. RE_POSSIBLE_UNINTENDED_PATTERN
Bug: "." used for regular expression Pattern id: RE_POSSIBLE_UNINTENDED_PATTERN, type: RE, category: CORRECTNESS
解釋:
String的split方法傳遞的參數是正則表達式,正則表達式本身用到的字符需要轉義,如:句點符號“.”,美元符號“$”,乘方符號“^”,大括號“{}”,方括號“[]”,圓括號“()” ,豎線“|”,星號“*”,加號“+”,問號“?”等等,這些需要在前面加上“\\”轉義符。
解決方法:
在前面加上“\\”轉義符。
13.IA_AMBIGUOUS_INVOCATION_OF_INHERITED_OR_OUTER_METHOD
外部類:
內部類:
……
Bug: Ambiguous invocation of either an outer or inherited method JExtendDialog.onOK() Pattern id: IA_AMBIGUOUS_INVOCATION_OF_INHERITED_OR_OUTER_METHOD, type: IA, category: STYLE
解釋:
TargetSetupDialog是JExtendDialog的子類,JExtendDialog有一個onOK方法,但是JExtendDialog的外部類也有一個onOK方法,到底這個onOK方法調用的是它父類onOK方法還是調用它外部類onOK方法呢,這不免讓人誤解。
當然這並沒有編譯錯誤,實際上優先調用的是父類JExtendDialog的onOK方法,如果把JExtendDialog的onOK方法去掉,它調用的就是外部類onOK方法,這個時候不能寫成this.onOK,因為此時的this並不代表外部類對象。
解決方法:
如果要引用外部類對象,可以加上“outclass.this”。
如果要引用父類的onOK方法,請使用super.onOK()。
14. DM_FP_NUMBER_CTOR
Bug: Method OnlineLicenseDAOTest.testUpdateOnlineLicenseByOnlineMerchantId() invokes inefficient Double.valueOf(double) constructor; use OnlineLicenseDAOTest.java:[line 81] instead Pattern id: DM_FP_NUMBER_CTOR, type: Bx, category: PERFORMANCE
Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.
Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.
解釋:
采用new Ddouble(double)會產生一個新的對象,采用Ddouble.valueOf(double)在編譯的時候可能通過緩存經常請求的值來顯著提高空間和時間性能。
解決方法:
采用Ddouble.valueOf方法
類似的案例
15. CN_IMPLEMENTS_CLONE_BUT_NOT_CLONEABLE
Bug:AlarmSoundManager$SoundProperty defines clone() but doesn‘t implement Cloneable Pattern id: CN_IMPLEMENTS_CLONE_BUT_NOT_CLONEABLE, type: CN, category: BAD_PRACTICE
解釋:
SoundProperty類實現了clone方法,但是沒有實現Cloneable接口,當然這沒有任何問題,但是你應該知道你為什麽這麽做。
解決方法:
最好實現Cloneable接口
16. STCAL_INVOKE_ON_STATIC_DATE_FORMAT_INSTANCE
Bug: Call to method of static java.text.DateFormat Pattern id: STCAL_INVOKE_ON_STATIC_DATE_FORMAT_INSTANCE, type: STCAL, category: MT_CORRECTNESS
解釋:
TIME_FORMAT是一個DateFormat靜態變量,文檔中DateFormat不是線程安全(多個線程訪問一個類時,這些線程執行順序沒有統一的調度和約定,如果這個類的行為仍然是正確的,那麽這個類就是線程安全的。考慮vector的實現)的,如果多個線程同時訪問,會出現意料不到的情況,詳情參見Sun Bug #6231579和Sun Bug #6178997。
因此對於DateFormat、SimpleDateFormat、Calendar類對象不建議定義成靜態成員字段使用,同時對它們在多線程環境下的使用請一定要保證同步。
另外,多說一句,java為我們提供了很多的封裝手段,比如private關鍵字、內部類、全限定包名等等,我們要充分利用這些手段封裝信息,對外盡量提供最小集。關於靜態變量也是如此,就算是vector這種線程安全的類,在無狀態類中也可能存在並發的問題,參見:無狀態類在並發環境中絕對安全嗎?
解決方法:
修改類字段為對象字段,然後改為private,同時提供get方法,最後對get方法實現同步機制。
最好連對象字段也去掉,直接在方法裏使用,就不存在同步的問題了(不必考慮性能問題,而且DateFormat本身就不必作為對象的字段,我想這也是sun為什麽不把它實現為線程安全的了)。
17. SE_NO_SERIALVERSIONID
Bug: WindowHandlerManager$MySingleSelectionModel is Serializable; consider declaring a serialVersionUID Pattern id: SE_NO_SERIALVERSIONID, type: SnVI, category: BAD_PRACTICE
This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.
解釋:
實現了Serializable接口,卻沒有實現定義serialVersionUID字段,序列化的時候,我們的對象都保存為硬盤上的一個文件,當通過網絡傳輸或者其他類加載方式還原為一個對象時,serialVersionUID字段會保證這個對象的兼容性,考慮兩種情況:
1. 新軟件讀取老文件,如果新軟件有新的數據定義,那麽它們必然會丟失。
2. 老軟件讀取新文件,只要數據是向下兼容的,就沒有任何問題。
序列化會把所有與你要序列化對象相關的引用(包括父類,特別是內部類持有對外部類的引用,這裏的例子就符合這種情況)都輸出到一個文件中,這也是為什麽能夠使用序列化能進行深拷貝。這種序列化算法給我們的忠告是,不要把一些你無法確定其基本數據類型的對象引用作為你序列化的字段(比如JFrame),否則序列化後的文件超大,而且會出現意想不到的異常。
解決方法:
定義serialVersionUID字段
18.SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
Bug: ToStringComparator implements Comparator but not Serializable Pattern id: SE_COMPARATOR_SHOULD_BE_SERIALIZABLE, type: Se, category: BAD_PRACTICE
解釋:
ToStringComparator類實現了Comparator接口卻沒有實現Serializable接口,因為像TreeMap這種可序列化數據結構(它實現了Serializable接口)只有當比較器繼承了Serializable接口時,它才能被序列化。
解決方法:
實現Serializable接口並定義serialVersionUID字段
19. ES_COMPARING_STRINGS_WITH_EQ
Bug: Comparison of String objects using == or != Pattern id: ES_COMPARING_STRINGS_WITH_EQ, type: ES, category: BAD_PRACTICE
解釋:
你確定你已經了解string的全部了?
如果你不了解,請參考FX大神的博文:請別再拿“String s = new String("xyz");創建了多少個String實例”來面試了吧
那麽,接下來我就開始剝皮了: Object和StringBuilder的toString方法都是返回一個new String(),跟””不相等。
如果你之前是這樣的定義的:String name = “”;OK,它們處於同一個class常量池,跟””相等。
如果在這之前,你使用了String. Intern方法,你是高手,跟””相等。
如果你沒有意識到這些問題,卻仍然使用==和!=去比較字符串,那麽請不要告訴我是你手滑了= =!
解決方法:
老實使用equals方法吧,至少為了保持代碼的清晰性。
20. ES_COMPARING_STRINGS_WITH_EQ
Bug: Comparison of String parameter using == or != Pattern id: ES_COMPARING_PARAMETER_STRING_WITH_EQ, type: ES, category: BAD_PRACTICE
解釋:
跟前面的例子差不多,你如果不能確保propertyName來源於常量池,那麽用==比較沒有一點意義,難不成你告訴我這能提高性能? 如果有功夫為這點性能擔驚受怕,還不如花點時間去找找性能瓶頸。
解決方法:
使用equals方法
21. IM_AVERAGE_COMPUTATION_COULD_OVERFLOW
Bug: Computation of average could overflow Pattern id: IM_AVERAGE_COMPUTATION_COULD_OVERFLOW, type: IM, category: STYLE
解釋:
參照了Findbugs的解釋,(low+high)/2當平均數過大的時候(難道是超過了int最大值?)會溢出,會出現一個負值,此問題出現在早期實現的二進制搜索和歸並排序,但是已經被修復了。參見Joshua Bloch(google首席java架構師)widely publicized the bug pattern(需FQ).
解決方法:
建議使用無符號右移位運算符:use (low+high) >>> 1
22. SC_START_IN_CTOR
Bug: new AsyncCentral() invokes AsyncCentral$FireThread.start() Pattern id: SC_START_IN_CTOR, type: SC, category: MT_CORRECTNESS
解釋:
構造方法裏重啟新的線程,我還是第一次見過這樣寫的。
首先說明三點:
1. 對象的創建一般分兩步走,在堆上new對象操作,執行<init>方法(包含構造方法),為什麽我們開發人員看見的只有一步,那是因為JVM不想讓開發人員在這個過程中插上一腳,破壞對象的初始化流程。
2. 類的加載和初始化是由虛擬機保證同步的,但是對象的生成和初始化就沒有任何同步機制來保證了。
3. 構造器不能加synchronized,是一項程序語言設計上的選擇(見:JLS 8.8.3 Constructor Modifiers),正常情況下,是不需要加上synchronized,但不代表所有的情況都不要加上synchronized,更不能認為一個構造器隱含的就是一個synchronized。
那什麽時候構造方法需要同步呢?通常來說,<init>方法在生成對象的時候只被執行一次,一般new對象的操作可能因為JVM自身的關系保證原子性操作(自己臆測的,沒有任何根據),所以我們經常不用關心構造方法同步的問題。但是上述情況就不一樣了,在構造方法中新啟線程,如果AsyncCentral是一個狀態類,FireThread線程極有可能對AsyncCentral的狀態進行反復讀取和寫入,更嚴重的一種情況是,AsyncCentral有父類,極有可能在父類的構造方法還沒開始前,FireThread線程就已經開始執行並對AsyncCentral的狀態進行“破壞”了。這個時候,就有兩個線程來對AsyncCentral的狀態進行操作了(一個是執行<init>方法的線程,一個是FireThread線程),自然而然,就會存在同步的問題了。
多數時候,我們沒有發現,可能是AsyncCentral類沒有狀態,或者是時候未到,我想說的是,我們寫的大部分程序都存在同步的問題,本例子就是其中一個,值得我們好好思考。
另一種理解(覺得更靠譜,來自於Java.Concurrency.in.Practice)叫做“對象逃逸”,意思就是說在構造方法裏,this是可以訪問的到的,同一時間,FireThread線程而是可以訪問到this對象的,所以這時候this就從<init>方法線程逃逸到了FireThread線程中,這時候初始化就會存在並發問題。
解決方法:
不要再構造方法中新啟線程,可以提供init方法,其他方法根據實際情況而定。
23. EQ_SELF_USE_OBJECT
Bug: ManageItem defines equals(ManageItem) method and uses Object.equals(Object) Pattern id: EQ_SELF_USE_OBJECT, type: Eq, category: CORRECTNESS
解釋:
這是重載,不是覆蓋,除非你能保證其他人調用這個方法傳入的參數都是ManageItem 的,否則會調用Object的boolean equals(Object)方法,這樣的話根本就不會跑到這個方法裏來!!很多所謂的大牛都會犯這麽一個錯誤,我堅信這是你手滑了。
解決方法:
如果你想覆蓋父類的方法,請在上面加上@Override註解,它會防止這種錯誤的出現(透露一個小細節,JDK1.5覆蓋接口方法時加上@Override編譯器會報錯,JDK1.6修正,這可能是當初實現者對@Override註解理解的問題)。
24.DLS_DEAD_LOCAL_STORE
案例二:
Bug: Dead store to date Pattern id: DLS_DEAD_LOCAL_STORE, type: DLS, category: STYLE
解釋:
先看看,我們的程序有多少個這樣的例子:
真是傷不起啊,不知道當時的作者這是神馬意圖?手滑?還是眼花?雖然說這不是神馬問題,也不會對程序性能造成多大的影響,但是這就像一顆沙子,我們每個程序員對待程序都應該是眼裏不能進沙子的態度,當然,你非要這麽寫,我也沒神馬可說的。
By the way:
對本地變量定義了之後未使用到,編譯器能夠做優化處理,也就是在編譯之後的class文件中刪除這些本地變量。方法是在eclipse的Preferences裏將以下的鉤去除:
解決方法:
大膽的去掉或者註釋掉。
誤報的案例:
上述案例二種: IntegralItemDO integralItem = new IntegralItemDO();
是一個局部的變量,不需要定義到外部去,定義在外部,可能會變成一個無效的變量。
25.FE_TEST_IF_EQUAL_TO_NOT_A_NUMBER
Bug: Doomed test for equality to NaN Pattern id: FE_TEST_IF_EQUAL_TO_NOT_A_NUMBER, type: FE, category: CORRECTNESS
解釋:
我也開眼界了,照搬Findbugs的理解:
大概意思就是說Nan很特殊(表示未定義和不可表示的值),沒有任何值跟它相等,包括它自身,所以x == Double.NaN永遠返回false。
解決方法:
如果要檢查x是特殊的,不是一個數值,請用Double.isNaN(x)方法。
26. FI_EMPTY
Bug: FilterIPConfigDialog.finalize() is empty and should be deleted Pattern id: FI_EMPTY, type: FI, category: BAD_PRACTICE
解釋:
空的finalize方法,有什麽用?
根據JDK文檔, finalize() 是一個用於釋放非 Java 資源的方法。但是, JVM 有很大的可能不調用對象的finalize() 方法,因此很難證明使用該方法釋放資源是有效的。
解決方法:
刪除掉finalize方法
27.REC_CATCH_EXCEPTION
Bug: Exception is caught when Exception is not thrown Pattern id: REC_CATCH_EXCEPTION, type: REC, category: STYLE
解釋:
我覺得有點迷惑,有些catch (Exception e)並沒有被Findbugs捕捉到,開始以為它的意思是try catch裏沒有任何異常的產生,包括RuntimeException,但是後來我寫了例子證明並不是這麽回事。
總之,它的意思應該是說JVM對RuntimeException有統一的捕獲機制(一般都是打印異常棧信息,然後向外拋,沒有遇到Exception線程就死掉,EDT線程除外),你搞一個catch (Exception e)這樣也把RuntimeException就捕獲了。但是如果你的處理機制中沒有針對這些異常,那就可能有問題了。通常來說,很多應用程序都把異常記錄在日誌之中,但是我覺得也應該同時打印在調試屏幕中,這樣有利於開發人員調試。
比如上面的程序,假如發生了空指針異常,你只有去日誌中才能看到,這對我們調試人員來說很不方便的。
解決方法:
其實這樣寫也沒有問題(除非你有意),有時候我們確實需要捕獲RuntimeException,比如我們有一個批處理,這個任務很重要,必須保證某個任務出了問題不能影響其他的任務,這個時候就可以在for循環內捕獲RuntimeException,出現了異常還可以continue。
不過上面的例子最好再把異常信息打印到調試屏幕上。
28. DM_GC
Bug: DBExportTask2.exportDBRecords(DBExportProperty, String) forces garbage collection; extremely dubious except in benchmarking code Pattern id: DM_GC, type: Dm, category: PERFORMANCE
解釋:
有兩點:
1. System.gc()只是建議,不是命令,JVM不能保證立刻執行垃圾回收。
2. System.gc()被顯示調用時,很大可能會觸發Full GC。
GC有兩種類型:Scavenge GC和Full GC,Scavenge GC一般是針對年輕代區(Eden區)進行GC,不會影響老年代和永生代(PerGen),由於大部分對象都是從Eden區開始的,所以Scavenge GC會頻繁進行,GC算法速度也更快,效率更高。但是Full GC不同,Full GC是對整個堆進行整理,包括Young、Tenured和Perm,所以比Scavenge GC要慢,因此應該盡可能減少Full GC的次數。
解決方法:
去掉System.gc()
28. DP_DO_INSIDE_DO_PRIVILEGED
Bug: com.taobao.sellerservice.core.test.BaseTestJunit.autoSetBean() invokes reflect.Field.setAccessible(boolean), which should be invoked from within a doPrivileged block Pattern id: DP_DO_INSIDE_DO_PRIVILEGED, type: DP, category: BAD_PRACTICE
This code invokes a method that requires a security permission check. If this code will be granted security permissions, but might be invoked by code that does not have security permissions, then the invocation needs to occur inside a doPrivileged block.
此代碼調用一個方法,需要一個安全權限檢查。如果此代碼將被授予安全權限,但可能是由代碼不具有安全權限調用,則需要調用發生在一個doPrivileged的塊。
30. MS_SHOULD_BE_FINAL
Bug: IPv4Document.m_strInitString isn‘t final but should be Pattern id: MS_SHOULD_BE_FINAL, type: MS, category: MALICIOUS_CODE
解釋:
使用public和protected,別的包可以輕易修改它,如果你不想它被修改,請使用final。
封裝很重要,不管是從維護方面和技術方面來說,都很重要,我不明白為神馬有那麽多的人把變量都寫成public的(就算要給別人共享,也要提供get方法),特別是在並發環境中,特別特別註意類變量的共享,而且特別特別特別註意共享的這個變量是否是線程安全的。
解決方法:
加上final
31. NM_FIELD_NAMING_CONVENTION
Bug: The field name TopoControlPaneII.SyncSelection doesn‘t start with a lower case letter Pattern id: NM_FIELD_NAMING_CONVENTION, type: Nm, category: BAD_PRACTICE
解釋:
為神馬字段是大寫開頭的?餵神馬?餵神馬啊?
解決方法:
建議按照sun規定的命名方式
Bug: Field only ever set to null: RaisecomStatus.infoURL Pattern id: UWF_NULL_FIELD, type: UwF, category: CORRECTNESS
解釋:
字段infoURL整個過程中一直為null,但卻被用來作為分支判斷的條件,不知道作者何意?難道真的是傳說中的手滑?
解決方法:
這個就要問作者的意圖了,當時你究竟要幹神馬來著?
32. MS_PKGPROTECT
Bug: ActionPatternManager.m_This should be package protected Pattern id: MS_PKGPROTECT, type: MS, category: MALICIOUS_CODE
解釋:
Findbugs說,靜態字段m_oThis應該是包權限的,如果是protected的話,可以被其他包訪問到,其實個人覺得僅僅是封裝範圍的話是一個“小問題”,畢竟很多人都沒意識到public、protected等關鍵字的重要性。但是我接著往下看:
單例模式??這是神馬單例模式?字段不是private,還是單例模式嗎?我在任何一個地方繼承UserManager,然後直接m_oThis = new UserManager();這還是一個單例嗎?
在看看Findbugs為我們找出了多少個:
另外,我很客觀的說一點,我們後怕,因為知道了真相,在想想我們實際情況中遇到很多不能復現的問題,我們有理由去知道這一切。
解決方法:
修改protected為private,然後將單例模式實現方式改為惡漢,或者雙重校驗鎖定。
33. FI_USELESS
Pattern: Finalizer does nothing but call superclass finalizer id: FI_USELESS, type: FI, category: BAD_PRACTICE
解釋:
finalize() 是一個用於釋放非 Java 資源的方法,這裏的finalize直接用Object的finalize方法,無任何意義。
解決方法:
勇敢去掉finalize()
34. NP_NULL_ON_SOME_PATH
Bug: Possible null pointer dereference of busCatId Pattern id: NP_NULL_ON_SOME_PATH, type: NP, category: CORRECTNESS
There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed. Of course, the problem might be that the branch or statement is infeasible and that the null pointer exception can‘t ever be executed; deciding that is beyond the ability of FindBugs.
解釋:
方法中存在空指針
解決方法:
增加字段busCatId為空的判斷
35. NP_NULL_ON_SOME_PATH
Bug:.HierarchicalManagerImpl.isExistByName(String, long) forgets to throw new exception.HierarchicalServiceException(String, Throwable) Pattern id: RV_EXCEPTION_NOT_THROWN, type: RV, category: CORRECTNESS
This code creates an exception (or error) object, but doesn‘t do anything with it. For example, something like
if (x < 0)
new IllegalArgumentException("x must be nonnegative");
It was probably the intent of the programmer to throw the created exception:
if (x < 0)
throw new IllegalArgumentException("x must be nonnegative");
解釋:
此代碼創建了一個異常(或錯誤)的對象,但並不做任何事情。
可能作者是想繼續拋出異常信息吧,可是卻產生了一個對象,啥也不幹。
解決方法:
拋出這個錯誤
36. FI_FINALIZER_NULLS_FIELDS
Bug: CustomerResTreeDialog.java:[line 67] is set to null inside finalize method Pattern id: FI_FINALIZER_NULLS_FIELDS, type: FI, category: BAD_PRACTICE
解釋:
關於finalize方法,前面應該已經介紹過了,所以m_UniResTree = null,純粹是多此一舉,沒有任何意義。
解決方法:
勇敢去掉finalize()
37. FI_PUBLIC_SHOULD_BE_PROTECTED
Bug: FilterIPConfigDialog.finalize() is public; should be protected Pattern id: FI_PUBLIC_SHOULD_BE_PROTECTED, type: FI, category: MALICIOUS_CODE
解釋:
Finalize方法不是protected的,當然你寫成public也沒錯,依然可以覆蓋父類中的finalize方法。
解決方法:
勇敢去掉finalize()
38. IS2_INCONSISTENT_SYNC
Bug: Inconsistent synchronization of URLAlarmMonitor.m_Counter; locked 50% of time Pattern id: IS2_INCONSISTENT_SYNC, type: IS, category: MT_CORRECTNESS
解釋:
m_Counter只鎖住了50%,它還是處於線程不安全的狀態,如果一個字段只被read,那麽它是線程安全的,不需要提供額外的同步開銷,可以定義為final的(參考不可變類的實現),如果既有read也有write,那麽就必須保證每個get和set方法都同步,而不能像上面一樣,只對set方法進行了同步。
解決方法:
對get和set方法都實行同步。
39. LI_LAZY_INIT_UPDATE_STATIC
Bug: Incorrect lazy initialization and update of static field MonitorRuleManager.m_This Pattern id: LI_LAZY_INIT_UPDATE_STATIC, type: LI, category: MT_CORRECTNESS
解釋:
此問題的m_This也是protected的,這裏就不再追究了。這裏的問題是,當線程1執行到initMonitorRules方法時,線程2執行getInstance方法,它直接返回m_This,這時候它可以用m_This做任何事情,但是此時線程1的初始化動作還沒有完成,如果initMonitorRules方法裏有對對象狀態進行更新的操作,那麽很可能線程2得到的對象的狀態是還沒有初始化的,這就是一個多線程的BUG(多線程的問題之所以很嚴重,是因為我們很難復現解決它,但它又是的確存在的,它總是在關鍵時候爆發,讓你感到很郁悶)!
當然就算沒有initMonitorRules方法,這個單例模式也不是線程安全的,下面會講到這個問
題。
解決方法:
將initMonitorRules方法放在構造方法裏,然後將單例改成惡漢模式,或者使用雙重校驗鎖。
40. LI_LAZY_INIT_STATIC
Bug: Incorrect lazy initialization of static field TopoController.m_This Pattern id: LI_LAZY_INIT_STATIC, type: LI, category: MT_CORRECTNESS
解釋:
為什麽它存在多線程的bug,比如線程1進入到if語句內,被線程2打斷,線程2同樣進入了if語句內然後生成了一個對象a,隨即被線程1打斷,線程1又生成了另外一個對象b,這還是一個單例麽?
更詳細的解釋請看:雙重檢查鎖定以及單例模式
另外,關於單例模式更多的資料,參見單例模式的七種寫法
如果你並發功底相當好,請看這篇文章:用happen-before規則重新審視DCL
解決方法:
我比較鐘情於惡漢,如果需要傳遞參數,我會使用雙重校驗鎖。
41. WMI_WRONG_MAP_ITERATOR
案例一:
案例二:
Bug: Method JTAMainFrame.initView(JFrame) makes inefficient use of keySet iterator instead of entrySet iterator Pattern id: WMI_WRONG_MAP_ITERATOR, type: WMI, category: PERFORMANCE
This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.
解釋:
很多人都這樣遍歷Map,沒錯,但是效率很低,先一個一個的把key遍歷,然後在根據key去查找value,這不是多此一舉麽,為什麽不遍歷entry(桶)然後直接從entry得到value呢?它們的執行效率大概為1.5:1(有人實際測試過)。
我們看看HashMap.get方法的源代碼: 1. public V get(Object key) { 2. if (key == null) 3. return getForNullKey(); 4. int hash = hash(key.hashCode()); 5. for (Entry<K,V> e = table[indexFor(hash, table.length)]; 6. e != null; 7. e = e.next) { 8. Object k; 9. if (e.hash == hash && ((k = e.key) == key || key.equals(k))) 10. return e.value; 11. } 12. return null; 13. }
從這裏可以看出查找value的原理,先計算出hashcode,然後散列表裏取出entry,不管是計算hashcode,還是執行循環for以及執行equals方法,都是CPU密集運算,非常耗費CPU資源,如果對一個比較大的map進行遍歷,會出現CPU迅速飈高的現象,直接影響機器的響應速度,在並發的情況下,簡直就是一場災難。
解決方法: 1. for (Map.Entry<String, JMenu> entry : menuList.entrySet()) { 2. mb.add(entry.getValue());
}
for(Map.Entry<String, List<BlackListDO>> tempEntiy: companyBlackItemsMap.entrySet()) {
String key = tempEntiy.getKey();
List<BlackListDO> eachCompanyBlackItems = tempEntiy.getValue();
42. BC_VACUOUS_INSTANCEOF
Bug: instanceof will always return true, since all TopoTreeNode are instances of TopoTreeNode Pattern id: BC_VACUOUS_INSTANCEOF, type: BC, category: STYLE
解釋:
因為getSelectedTreeNode方法返回類型就是TopoTreeNode,所以if裏的instanceof測試永遠為true,除非它是null,確保你沒有其他的邏輯上的誤解,你這樣寫,會讓其他人丈二和尚摸不著頭腦。
解決方法:
去掉instanceof檢測。
43. INT_BAD_REM_BY_1
Bug: Integer remainder modulo 1 computed Pattern id: INT_BAD_REM_BY_1, type: INT, category: STYLE
解釋:
I % 1永遠都為0,I / 1也為i,不知道作者想幹嘛。
解決方法:
恕我愚昧,不明白作者的意圖。
Bug: Load of known null value Pattern id: NP_LOAD_OF_KNOWN_NULL_VALUE, type: NP, category: STYLE
The variable referenced at this point is known to be null due to an earlier check against null. Although this is valid, it might be a mistake (perhaps you intended to refer to a different variable, or perhaps the earlier check to see if the variable is null should have been a check to see if it was nonnull).
解釋:
Node為null,還進一步調用它上面的方法,除非你能保證當node為null的時候isDeleteSingleObject為false,否則很可能發生空指針異常,我估計作者是第二個if是想判斷node != null吧。
解決方法:
努力找到原作者,當面詢問其用意。
44. EI_EXPOSE_REP2
案例
DO類
Bug: SingleNePollConfigDialog.collectValues(Hashtable) may expose internal representation by storing an externally mutable object into SingleNePollConfigDialog.values Pattern id: EI_EXPOSE_REP2, type: EI2, category: MALICIOUS_CODE
解釋:
參數values保存在當前線程的執行棧中,而this.values保存在堆上,它們同時指向同一個對象,對參數values的任何操作都會影響到this.values,如果你知道這一點,而且本意就是這樣的,那麽你可以忽略上面這些話,但是下面這些話你應該好好聽聽。
這是一段正確的代碼,但不是一段可維護性強、可理解性強的代碼,參數代表操作的條件,它們應該是只讀的,我們不應該對它直接進行操作或者賦值。
解決方法:
如果把上面對參數values的操作都改成this.values,我相信你和你的同事都會覺得這樣的代碼更加清晰。
}
案例二
DO類
Bug: SingleNePollConfigDialog.collectValues(Hashtable) may expose internal representation by storing an externally mutable object into SingleNePollConfigDialog.values Pattern id: EI_EXPOSE_REP2, type: EI2, category: MALICIOUS_CODE
This code stores a reference to an externally mutable object into the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
翻譯願意:
此代碼存儲到一個到對象的內部表示外部可變對象的引用。如果實例是由不受信任的代碼,並以可變對象會危及安全或其他重要的屬性選中更改訪問,你需要做不同的東西。存儲一個對象的副本,在許多情況下是更好的辦法。
解釋:
DO類實例產生之後,裏面包含的Date不是原始數據類型,導致其gmtCrate屬性不光是set方法可以改變其值,外部引用修改之後也可能導致gmtCreate 被改變,會引起可能的不安全或者錯誤。
這個是一個不好的實踐,不過我們應用裏面DO都是比較簡單使用,不太會出現這種情況。
解決方法:
修改成:
public Date getGmtCreate() { return new Date(this.gmtCreate.getTime()); //正確值
}
45. EI_EXPOSE_REP
Bug: temsLoader.getItemsWithPriority() may expose internal representation by returning ItemsLoader.m_htItemsWithPriority Pattern id: EI_EXPOSE_REP, type: EI, category: MALICIOUS_CODE
解釋:
剛開始一看挺納悶的,這個方法有什麽問題嗎?後來仔細看一下,發現返回值都有一個特點,它們都是集合數組之類的,我想findBugs的本意是,某些數據集合不應該直接對外提供public返回方法,即使表面上提供了get方法,但實際上可以任意修改裏面的數據。
解決方法:
如果你確定這些數據集合不應該被外界修改,那麽對於基本數據類型,你提供get方法即可,對於引用,get方法裏的返回值應該是數據的拷貝。
46. NP_NULL_PARAM_DEREF
Bug: Method call passes null for nonnull parameter of queryScriptData(ObjService) Pattern id: NP_NULL_PARAM_DEREF, type: NP, category: CORRECTNESS
解釋:
當getAllListFiles方法發生了任何異常(checked和unchecked),allFiles都為null,關鍵是在queryScriptData方法裏,並沒有對參數是否為null進行判斷,它直接調用了參數對象上面的方法,這肯定會發生空指針異常。
一個優秀的程序員,在過馬路時都要向兩邊看一下,在寫一個方法時,首先要考慮的就是對方法參數的有效性判斷。
解決方法:
在queryScriptData方法裏對參數進行有效性判斷。
46. SBSC_USE_STRINGBUFFER_CONCATENATION
Bug: Method InitDBPoolParaTask.execute() concatenates strings using + in a loop Pattern id: SBSC_USE_STRINGBUFFER_CONCATENATION, type: SBSC, category: PERFORMANCE
The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.
Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 1.5) explicitly.
解釋:
每次循環裏的字符串+連接,都會新產生一個string對象,在java中,新建一個對象的代價是很昂貴的,特別是在循環語句中,效率較低。
解決方法:
利用StringBuffer或者StringBuilder重用對象。
47. RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
Bug: NewScriptAction.actionPerformed(ActionEvent) ignores exceptional return value of java.io.File.delete() Pattern id: RV_RETURN_VALUE_IGNORED_BAD_PRACTICE, type: RV, category: BAD_PRACTICE
解釋:
關於一個方法邏輯執行是否成功,有兩種方式,一種是拋出異常,一種是提供boolean類型的返回值。舉一個例子,用戶登錄,某些人將login方法的返回值定義為int,然後枚舉出各個值的含義,比如0代表成功,1代表用戶名不存在等等;而有些人,把這些枚舉值看成是use case中的異常流,將它們定義為異常對象,遇到“異常”情況直接拋出異常從而實現分支的流程。第一種方式是典型的C語言面向過程風格,第二種方式,帶有強烈的面向對象味道,特別是java提供了checked Exception,貌似偏離主題了。
java中很多方法的執行成功依賴於異常的分支實現,但也有提供返回值的實現,比如這裏的File.delete方法,上面的寫法忽略了返回值(如果調用某個方法卻不使用其返回值要特別註意),刪除一個文件很可能不成功,但是從代碼裏並沒有看到這一層面的意思。
解決方法:
文件刪除不成功該怎麽辦?現在能處理就處理,現在不能處理就把父類的方法也改成有返回值的,然後向上傳遞,這跟處理異常的道理是一樣的,當然,你也可以把它封裝成一個異常對象。
48. RV_RETURN_VALUE_IGNORED
Bug: BackupFileListPanel$PopupListener.maybeShowPopup(MouseEvent) ignores return value of String.trim() Pattern id: RV_RETURN_VALUE_IGNORED, type: RV, category: CORRECTNESS
解釋:
String是一個不可變類,調用String上的任何操作都會返回新的String對象,雖然String是一個class,但實際上對它的任何操作都可以把它看成基本數據類型,比如s.trim方法是不會改變s值的。
解決方法:
S = s.trim
49. DM_BOOLEAN_CTOR
Bug: TopoCardManagerAction.processLocalCard(Hashtable) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead Pattern id: DM_BOOLEAN_CTOR, type: Dm, category: PERFORMANCE
解釋:
不必創建一個新的Boolean對象,使用Boolean.valueOf方法可以重用Boolean.FALSE和Boolean.TRUE對象。
我們可以從API中可以看到public Boolean(boolean value)方法的解釋:註:一般情況下都不宜使用該構造方法。若不需要新 的實例,則靜態工廠 valueOf(boolean) 通常是一個更好的選擇。這有可能顯著提高空間和時間性能。
解決方法:
使用Boolean.valueOf方法或者直接返回Boolean.FALSE和Boolean.TRUE對象。
50. RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
Pattern id: RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE,
解釋:
StringBuffer連接更有効率,因此,建議使用StringBuffer
51. DM_NUMBER_CTOR
new Integer(int) 和 Integer.valueOf(int)
bug描述:
[Bx] Method invokes inefficient Number constructor; use static valueOf instead [DM_NUMBER_CTOR]
Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.
說明:
[參考]http://www.cnblogs.com/hyddd/articles/1391318.html
FindBugs推薦使用Integer.ValueOf(int)代替new Integer(int),因為這樣可以提高性能。如果當你的int值介於-128~127時,Integer.ValueOf(int)的效率比Integer(int)快大約3.5倍。
下面看看JDK的源碼,看看到Integer.ValueOf(int)裏面做了什麽優化:
public static Integer valueOf(int i) { final int offset = 128; if (i >= -128 && i <= 127) { // must cache return IntegerCache.cache[i + offset]; } return new Integer(i); } private static class IntegerCache { private IntegerCache(){} static final Integer cache[] = new Integer[-(-128) + 127 + 1]; static { for(int i = 0; i < cache.length; i++) cache = new Integer(i - 128); } }
從源代碼可以知道,ValueOf對-128~127這256個值做了緩存(IntegerCache),如果int值的範圍是:-128~127,在ValueOf(int)時,他會直接返回IntegerCache的緩存給你。
所以你會看到這樣的一個現象:
public static void main(String []args) { Integer a = 100; Integer b = 100; System.out.println(a==b); Integer c = new Integer(100); Integer d = new Integer(100); System.out.println(c==d); }
結果是:
true false
因為:java在編譯的時候 Integer a = 100; 被翻譯成-> Integer a = Integer.valueOf(100);,所以a和b得到都是一個Cache對象,並且是同一個!而c和d是新創建的兩個不同的對象,所以c自然不等於d。
再看看這段代碼:
public static void main(String args[]) throws Exception{ Integer a = 100; Integer b = a; a = a + 1; //或者a++; System.out.println(a==b); }
結果是:false

因為在對a操作時(a=a+1或者a++),a重新創建了一個對象,而b對應的還是緩存裏的100,所以輸出的結果為false。


原文地址:http://www.cnblogs.com/java-zone/articles/3118592.html

FindBugs錯誤修改指南

FindBugs錯誤修改指南 【轉】