1. 程式人生 > >關於java程式碼質量的問題

關於java程式碼質量的問題

一、

錯誤碼:WMI_WRONG_MAP_ITERATOR
案例一: 

54bde495-1b3f-39b5-86fa-43401d2e2ec0.jpg

案例二: 

9854ef6f-adf3-3a21-abd4-64c754a08d14.jpg


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方法的原始碼: 

Java程式碼  收藏程式碼

  1. 1.  public V get(Object key) {    

  2. 2.      if (key == null)    

  3. 3.          return getForNullKey();    

  4. 4.      int hash = hash(key.hashCode());    

  5. 5.      for (Entry<K,V> e = table[indexFor(hash, table.length)];    

  6. 6.           e != null;    

  7. 7.           e = e.next) {    

  8. 8.          Object k;    

  9. 9.          if (e.hash == hash && ((k = e.key) == key || key.equals(k)))    

  10. 10.             return e.value;    

  11. 11.     }    

  12. 12.     return null;    

  13. 13. }    


從這裡可以看出查詢value的原理,先計算出hashcode,然後散列表裡取出entry,不管是計算hashcode,還是執行迴圈for以及執行equals方法,都是CPU密集運算,非常耗費CPU資源,如果對一個比較大的map進行遍歷,會出現CPU迅速飈高的現象,直接影響機器的響應速度,在併發的情況下,簡直就是一場災難。 
解決方法:

Java程式碼  收藏程式碼

  1. 1.  for (Map.Entry<String, JMenu> entry : menuList.entrySet()) {    

  2. 2.      mb.add(entry.getValue());  

  3. }  





d58db580-0fcf-3697-9c68-7b9e82e0afa4.jpg




錯誤碼:EI_EXPOSE_REP2

案例 

663bc320-8b21-3862-9715-3ae21c812da9.jpg

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屬性不光DO例項的set方法可以改變其值,外部引用修改之後也可能導致gmtCreate 被改變,會引起可能的不安全或者錯誤。 
這個是一個不好的實踐,不過我們應用裡面DO都是比較簡單使用,不太會出現這種情況。 

解決方法: 
修改成: 

Java程式碼  收藏程式碼

  1. public Date getGmtCreate() {  

  2.         if(this.gmtCreate != null)  

  3.            return new Date(this.gmtCreate.getTime()); //正確值  

  4.         else   

  5.             return null;  

  6. }  



總結:這個其實是說可變類和不可變類的問題, 
可變類:當你獲得這個類的一個例項引用時,你可以改變這個例項的內容。 
不可變類:當你獲得這個類的一個例項引用時,你不可以改變這個例項的內容。不可變類 的例項一但建立,其內在成員變數的值就不能被修改 ; 
DO是一個可變類,但是最好是隻提供set方法才能改變其例項的的成員變數的值,減少被修改的風險。 

  • 54bde495-1b3f-39b5-86fa-43401d2e2ec0-thu

  • 9854ef6f-adf3-3a21-abd4-64c754a08d14-thu

  • d58db580-0fcf-3697-9c68-7b9e82e0afa4-thu

  • 663bc320-8b21-3862-9715-3ae21c812da9-thu

  • 561f4747-6a45-3b9c-9c2a-98d6a1b13a03-thu

  • 8769da4f-fc2b-3f68-a145-c4b5bc39fb3c-thu

  • 二、

  • 錯誤碼:DM_FP_NUMBER_CTOR 
    5e977a03-02fd-34d8-ba0f-6d157b507371.jpg 

    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方法 

    類似的案例 



    37e7b373-56db-3176-8a46-e2137eb82cd4.jpg 

    類似的還有: 
    錯誤碼: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)裡面做了什麼優化: 

  • Java程式碼  收藏程式碼

  • 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的快取給你。 
    所以你會看到這樣的一個現象: 

  • Java程式碼  收藏程式碼

  • 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。 
    再看看這段程式碼: 

  • Java程式碼  收藏程式碼

  • 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。 



    錯誤碼:DM_BOOLEAN_CTOR 
    def3566f-e95a-319a-926a-c5c6811d1728.jpg 

    Bug: Bad attempt to compute absolute value of signed 32-bit hashcode 
    Pattern id: RV_ABSOLUTE_VALUE_OF_HASHCODE, type: RV, category: CORRECTNESS 
    This code generates a hashcode and then computes the absolute value of that hashcode. If the hashcode is Integer.MIN_VALUE, then the result will be negative as well (since Math.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE). 
    One out of 2^32 strings have a hashCode of Integer.MIN_VALUE, including "polygenelubricants" "GydZG_" and ""DESIGNING WORKHOUSES". 
    解釋: 
    此程式碼產生的雜湊碼,然後計算該雜湊碼的絕對值。如果雜湊碼是Integer.MIN_VALUE的,那麼結果將是負面的,以及(因為Math.abs(Integer.MIN_VALUE的)==Integer.MIN_VALUE的)。 

    解決方法: 
    在使用之前判斷一下是否是為Integer.MIN_VALUE 

  • Java程式碼  收藏程式碼

  • int iTemp = sellerId.hashCode();  

  •        if(iTemp != Integer.MIN_VALUE) {  

  •          number = Math.abs(iTemp) % 12;  

  • else {  

  •        number = Integer.MIN_VALUE % 12;  

  • }  

  • 5e977a03-02fd-34d8-ba0f-6d157b507371-thu

  • 37e7b373-56db-3176-8a46-e2137eb82cd4-thu

  • def3566f-e95a-319a-926a-c5c6811d1728-thu

  • 三、

  • 錯誤碼:SE_NO_SERIALVERSIONID


    8ee11461-b410-3bf7-82fb-0ec66e37fc14.jpg


    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欄位 


    錯誤碼:DM_GC

    176e8af5-ba75-393e-a052-93103b1c90cf.jpg

    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() 



    錯誤碼:DP_DO_INSIDE_DO_PRIVILEGED 

    cd0e5a93-b33e-3aa6-bd27-aa604c11bb12.jpg

    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的塊。 
    解決方法:一般情況下,我們並不能對類的私有欄位進行操作,利用反射也不例外,但有的時候,例如要序列化的時候,我們又必須有能力去處理這些欄位,這時候,我們就需要呼叫AccessibleObject上的setAccessible()方法來允許這種訪問,而由於反射類中的Field,Method和Constructor繼承自AccessibleObject,因此,通過在這些類上呼叫setAccessible()方法,我們可以實現對這些欄位的操作。但有的時候這將會成為一個安全隱患,為此,我們可以啟用java.security.manager來判斷程式是否具有呼叫setAccessible()的許可權。預設情況下,核心API和擴充套件目錄的程式碼具有該許可權,而類路徑或通過URLClassLoader載入的應用程式不擁有此許可權。例如:當我們以這種方式來執行上述程式時將會丟擲異常 
    增加:} catch (SecurityException e) { 


    錯誤碼:NP_NULL_ON_SOME_PATH

    60b587eb-a94b-3edf-9afd-a0f533a37caf.jpg

    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為空的判斷 

  • 176e8af5-ba75-393e-a052-93103b1c90cf-thu

  • 60b587eb-a94b-3edf-9afd-a0f533a37caf-thu

  • cd0e5a93-b33e-3aa6-bd27-aa604c11bb12-thu

  • 8ee11461-b410-3bf7-82fb-0ec66e37fc14-thu

  • 四、

錯誤碼:RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 
  

d309c0e6-d1d2-35d1-9e01-093748dc45fe.jpg 

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. 
解釋:這種方法包含了一個稱為非空對空值的不斷重複檢查。 
修改為: 


8644fd8c-1e2d-3f90-99b6-4aef922af903.jpg 



錯誤碼:SS_SHOULD_BE_STATIC 


d62f4cf0-bed3-3181-865f-95c8e652cf68.jpg 

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關鍵字 



錯誤碼:NM_METHOD_NAMING_CONVENTION 

69b8122f-e464-35b9-a5fd-c3e59320d3ae.jpg 

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. 
解釋: 
方法應該是動詞,與第一個字母小寫混合的情況下,與每個單詞的首字母大寫的內部。 
解決方法: 
方法名稱小寫就通過了。我們寫程式碼還是要規範啊,雖然是單元測試的程式碼! 

  • d309c0e6-d1d2-35d1-9e01-093748dc45fe-thu

  • 8644fd8c-1e2d-3f90-99b6-4aef922af903-thu

  • d62f4cf0-bed3-3181-865f-95c8e652cf68-thu

  • 69b8122f-e464-35b9-a5fd-c3e59320d3ae-thu

  • 五、

  • REC_CATCH_EXCEPTION 


    0226083e-a04e-35d1-a62c-36bd3321b683.jpg

    Bug: Exception is caught when Exception is not thrown 
    Pattern id: REC_CATCH_EXCEPTION, type: REC, category: STYLE


    This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs. 

    這想寫會無意中把RuntimeException也捕獲了,有可能導致潛在的bug。 JVM對RuntimeException有統一的捕獲機制,讓JVM來處理它。 

    在try/catch塊中捕獲異常,但是異常沒有在try語句中丟擲而RuntimeException又沒有明確的被捕獲 

    1.比較推薦的寫法一般如下: 

    2.捕獲了異常,一定要處理異常 
    還有人在catch裡面什麼都不寫,就寫上 

    3.避免在大的語句塊裡面寫try,catch,因為本身也比較耗費時間,而