代碼審查是消滅Bug最重要的方法之一，這些審查在大多數時(shí)候都特別奏效。由于代碼審查本身所針對的對象，就是俯瞰整個(gè)代碼在測試過(guò)程中的問(wèn)題和Bug。并且，代碼審查對消除一些特別細節的錯誤大有裨益，尤其是那些能夠容易在閱讀代碼的時(shí)候發(fā)現的錯誤，這些錯誤往往不容易通過(guò)機器上的測試識別出來(lái)。本文就常見(jiàn)的Java代碼中容易出現的問(wèn)題提出一些建設性建議，以便您在審查代碼的過(guò)程中注意到這些常見(jiàn)的細節性錯誤。
通常給別人的工作挑錯要比找自己的錯容易些。別樣視角的存在也解釋了為什么作者需要編輯，而運動(dòng)員需要教練的原因。不僅不應當拒絕別人的批評，我們應該歡迎別人來(lái)發(fā)現并指出我們的編程工作中的不足之處，我們會(huì )受益匪淺的。
正規的代碼審查(code inspection)是提高代碼質(zhì)量的最強大的技術(shù)之一，代碼審查—由同事們尋找代碼中的錯誤—所發(fā)現的錯誤與在測試中所發(fā)現的錯誤不同，因此兩者的關(guān)系是互補的，而非競爭的。
如果審查者能夠有意識地尋找特定的錯誤，而不是靠漫無(wú)目的的瀏覽代碼來(lái)發(fā)現錯誤，那么代碼審查的效果會(huì )事半功倍。在這篇文章中，我列出了11個(gè)Java編程中常見(jiàn)的錯誤。你可以把這些錯誤添加到你的代碼審查的檢查列表(checklist)中，這樣在經(jīng)過(guò)代碼審查后，你可以確信你的代碼中不再存在這類(lèi)錯誤了。
常見(jiàn)錯誤1# ：多次拷貝字符串
測試所不能發(fā)現的一個(gè)錯誤是生成不可變(immutable)對象的多份拷貝。不可變對象是不可改變的，因此不需要拷貝它。最常用的不可變對象是String。
如果你必須改變一個(gè)String對象的內容，你應該使用StringBuffer。下面的代碼會(huì )正常工作：
String s = new String ("Text here");
但是，這段代碼性能差，而且沒(méi)有必要這么復雜。你還可以用以下的方式來(lái)重寫(xiě)上面的代碼：
String s = new String (temp);String temp = "Text here";
但是這段代碼包含額外的String，并非完全必要。更好的代碼為：
String s = "Text here";
常見(jiàn)錯誤2#： 沒(méi)有克隆(clone)返回的對象
封裝(encapsulation)是面向對象編程的重要概念。不幸的是，Java為不小心打破封裝提供了方便——Java允許返回私有數據的引用(reference)。下面的代碼揭示了這一點(diǎn)：
/***Example class.The x and y values should never*be negative.*/ public class Example{ private Dimension d = new Dimension (0, 0); public Example (){ } /*** Set height and width. Both height and width must be nonnegative * or an exception is thrown.*/ public synchronized void setValues (int height,int width) throws IllegalArgumentException{ if (height < 0 || width < 0) throw new IllegalArgumentException(); d.height = height; d.width = width; } public synchronized Dimension getValues(){ // Ooops! Breaks encapsulation return d; } }import java.awt.Dimension;
Example類(lèi)保證了它所存儲的height和width值永遠非負數，試圖使用setValues()方法來(lái)設置負值會(huì )觸發(fā)異常。不幸的是，由于getValues()返回d的引用，而不是d的拷貝，你可以編寫(xiě)如下的破壞性代碼：
Dimension d = ex.getValues(); d.height = -5; d.width = -10;Example ex = new Example();
現在，Example對象擁有負值了！如果getValues() 的調用者永遠也不設置返回的Dimension對象的width 和height值，那么僅憑測試是不可能檢測到這類(lèi)的錯誤。
不幸的是，隨著(zhù)時(shí)間的推移，客戶(hù)代碼可能會(huì )改變返回的Dimension對象的值，這個(gè)時(shí)候，追尋錯誤的根源是件枯燥且費時(shí)的事情，尤其是在多線(xiàn)程環(huán)境中。
更好的方式是讓getValues()返回拷貝：
return new Dimension (d.x, d.y); }public synchronized Dimension getValues(){
現在，Example對象的內部狀態(tài)就安全了。調用者可以根據需要改變它所得到的拷貝的狀態(tài)，但是要修改Example對象的內部狀態(tài)，必須通過(guò)setValues()才可以。
常見(jiàn)錯誤3#：不必要的克隆
我們現在知道了get方法應該返回內部數據對象的拷貝，而不是引用。但是，事情沒(méi)有絕對：
public class Example{ private Integer i = new Integer (0); public Example (){ } /*** Set x. x must be nonnegative* or an exception will be thrown*/ public synchronized void setValues (int x) throws IllegalArgumentException{ if (x < 0) throw new IllegalArgumentException(); i = new Integer (x); } public synchronized Integer getValue(){ // We can’t clone Integers so we makea copy this way. return new Integer (i.intValue()); } }/*** Example class.The value should never * be negative.*/
這段代碼是安全的，但是就象在錯誤1#那樣，又作了多余的工作。Integer對象，就象String對象那樣，一旦被創(chuàng )建就是不可變的。因此，返回內部Integer對象，而不是它的拷貝，也是安全的。
方法getValue()應該被寫(xiě)為：
// ’i’ is immutable, so it is safe to return it instead of a copy. return i; }public synchronized Integer getValue(){
Java程序比C++程序包含更多的不可變對象。JDK 所提供的若干不可變類(lèi)包括：
·Byte ·Character ·Class ·Double ·Float ·Integer ·Long ·Short ·String ·大部分的Exception的子類(lèi)·Boolean
常見(jiàn)錯誤4# ：自編代碼來(lái)拷貝數組
Java允許你克隆數組，但是開(kāi)發(fā)者通常會(huì )錯誤地編寫(xiě)如下的代碼，問(wèn)題在于如下的循環(huán)用三行做的事情，如果采用Object的clone方法用一行就可以完成：
private int[] copy; /*** Save a copy of ’data’. ’data’ cannot be null.*/ public void saveCopy (int[] data){ copy = new int[data.length]; for (int i = 0; i < copy.length; ++i) copy[i] = data[i]; } }public class Example{
這段代碼是正確的，但卻不必要地復雜。saveCopy()的一個(gè)更好的實(shí)現是：
try{ copy = (int[])data.clone(); }catch (CloneNotSupportedException e){ // Can’t get here. } }void saveCopy (int[] data){
如果你經(jīng)?？寺到M，編寫(xiě)如下的一個(gè)工具方法會(huì )是個(gè)好主意：
try{ return(int[])data.clone(); }catch(CloneNotSupportedException e){ // Can’t get here. } }static int[] cloneArray (int[] data){
這樣的話(huà)，我們的saveCopy看起來(lái)就更簡(jiǎn)潔了：
copy = cloneArray ( data); }void saveCopy (int[] data){
常見(jiàn)錯誤5#：拷貝錯誤的數據
有時(shí)候程序員知道必須返回一個(gè)拷貝，但是卻不小心拷貝了錯誤的數據。由于僅僅做了部分的數據拷貝工作，下面的代碼與程序員的意圖有偏差：
/*** Example class. The height and width values should never * be negative. */ public class Example{ static final public int TOTAL_VALUES = 10; private Dimension[] d = new Dimension[TOTAL_VALUES]; public Example (){ } /*** Set height and width. Both height and width must be nonnegative * or an exception will be thrown. */ public synchronized void setValues (int index, int height, int width) throws IllegalArgumentException{ if (height < 0 || width < 0) throw new IllegalArgumentException(); if (d[index] == null) d[index] = new Dimension(); d[index].height = height; d[index].width = width; } public synchronized Dimension[] getValues() throws CloneNotSupportedException{ return (Dimension[])d.clone(); } }import java.awt.Dimension;
這兒的問(wèn)題在于getValues()方法僅僅克隆了數組，而沒(méi)有克隆數組中包含的Dimension對象，因此，雖然調用者無(wú)法改變內部的數組使其元素指向不同的Dimension對象，但是調用者卻可以改變內部的數組元素(也就是Dimension對象)的內容。方法getValues()的更好版本為：
throws CloneNotSupportedException{ Dimension[] copy = (Dimension[])d.clone(); for (int i = 0; i < copy.length; ++i){ // NOTE: Dimension isn’t cloneable. if (d[i] != null) copy[i] = new Dimension (d[i].height, d[i].width); } return copy; }public synchronized Dimension[] getValues()
在克隆原子類(lèi)型數據的多維數組的時(shí)候，也會(huì )犯類(lèi)似的錯誤。原子類(lèi)型包括int,float等。簡(jiǎn)單的克隆int型的一維數組是正確的，如下所示：
throws CloneNotSupportedException{ this.data = (int[])data.clone(); // OK }public void store (int[] data)
拷貝int型的二維數組更復雜些。Java沒(méi)有int型的二維數組，因此一個(gè)int型的二維數組實(shí)際上是一個(gè)這樣的一維數組：它的類(lèi)型為int[]。簡(jiǎn)單的克隆int[][]型的數組會(huì )犯與上面例子中g(shù)etValues()方法第一版本同樣的錯誤，因此應該避免這么做。下面的例子演示了在克隆int型二維數組時(shí)錯誤的和正確的做法：
throws CloneNotSupportedException{ this.data = (int[][])data.clone(); // Not OK! } public void rightStore (int[][] data){ // OK! this.data = (int[][])data.clone(); for (int i = 0; i < data.length; ++i){ if (data[i] != null) this.data[i] = (int[])data[i].clone(); } } 常見(jiàn)錯誤6#：檢查new 操作的結果是否為null Java編程新手有時(shí)候會(huì )檢查new操作的結果是否為null?？赡艿臋z查代碼為： if (i == null) throw new NullPointerException(); 檢查當然沒(méi)什么錯誤，但卻不必要，if和throw這兩行代碼完全是浪費，他們的唯一功用是讓整個(gè)程序更臃腫，運行更慢。 C/C++程序員在開(kāi)始寫(xiě)java程序的時(shí)候常常會(huì )這么做，這是由于檢查C中malloc()的返回結果是必要的，不這樣做就可能產(chǎn)生錯誤。檢查C++中new操作的結果可能是一個(gè)好的編程行為，這依賴(lài)于異常是否被使能(許多編譯器允許異常被禁止，在這種情況下new操作失敗就會(huì )返回null)。在java 中，new 操作不允許返回null，如果真的返回null，很可能是虛擬機崩潰了，這時(shí)候即便檢查返回結果也無(wú)濟于事。 常見(jiàn)錯誤7#：用== 替代.equals 在Java中，有兩種方式檢查兩個(gè)數據是否相等：通過(guò)使用操作符，或者使用所有對象都實(shí)現的.equals方法。原子類(lèi)型(int, flosat, char 等)不是對象，因此他們只能使用==操作符，如下所示： int y = 5; if (x == y) System.out.println ("Hi"); // This ’if’ test won’t compile. if (x.equals (y)) System.out.println ("Hi"); 對象更復雜些，==操作符檢查兩個(gè)引用是否指向同一個(gè)對象，而equals方法則實(shí)現更專(zhuān)門(mén)的相等性檢查。 更顯得混亂的是由java.lang.Object 所提供的缺省的equals方法的實(shí)現使用==來(lái)簡(jiǎn)單的判斷被比較的兩個(gè)對象是否為同一個(gè)。 許多類(lèi)覆蓋了缺省的equals方法以便更有用些，比如String類(lèi)，它的equals方法檢查兩個(gè)String對象是否包含同樣的字符串，而Integer的equals方法檢查所包含的int值是否相等。 大部分時(shí)候，在檢查兩個(gè)對象是否相等的時(shí)候你應該使用equals方法，而對于原子類(lèi)型的數據，你用該使用==操作符。 常見(jiàn)錯誤8#： 混淆原子操作和非原子操作 Java保證讀和寫(xiě)32位數或者更小的值是原子操作，也就是說(shuō)可以在一步完成，因而不可能被打斷，因此這樣的讀和寫(xiě)不需要同步。以下的代碼是線(xiàn)程安全(thread safe)的： private int value; // More code here... public void set (int x){ // NOTE: No synchronized keyword this.value = x; } } 不過(guò)，這個(gè)保證僅限于讀和寫(xiě)，下面的代碼不是線(xiàn)程安全的： // This is effectively two or three instructions: // 1) Read current setting of ’value’. // 2) Increment that setting. // 3) Write the new setting back. ++this.value; } 在測試的時(shí)候，你可能不會(huì )捕獲到這個(gè)錯誤。首先，測試與線(xiàn)程有關(guān)的錯誤是很難的，而且很耗時(shí)間。其次，在有些機器上，這些代碼可能會(huì )被翻譯成一條指令，因此工作正常，只有當在其它的虛擬機上測試的時(shí)候這個(gè)錯誤才可能顯現。因此最好在開(kāi)始的時(shí)候就正確地同步代碼： ++this.value; } 常見(jiàn)錯誤9#：在catch 塊中作清除工作 一段在catch塊中作清除工作的代碼如下所示： try{ os = new OutputStream (); // Do something with os here. os.close(); }catch (Exception e){ if (os != null) os.close(); } 盡管這段代碼在幾個(gè)方面都是有問(wèn)題的，但是在測試中很容易漏掉這個(gè)錯誤。下面列出了這段代碼所存在的三個(gè)問(wèn)題： 1．語(yǔ)句os.close()在兩處出現，多此一舉，而且會(huì )帶來(lái)維護方面的麻煩。 2．上面的代碼僅僅處理了Exception，而沒(méi)有涉及到Error。但是當try塊運行出現了Error，流也應該被關(guān)閉。 3．close()可能會(huì )拋出異常。 上面代碼的一個(gè)更優(yōu)版本為： try{ os = new OutputStream (); // Do something with os here. }finally{ if (os != null) os.close(); } 這個(gè)版本消除了上面所提到的兩個(gè)問(wèn)題：代碼不再重復，Error也可以被正確處理了。但是沒(méi)有好的方法來(lái)處理第三個(gè)問(wèn)題，也許最好的方法是把close()語(yǔ)句單獨放在一個(gè)try/catch塊中。 常見(jiàn)錯誤10#： 增加不必要的catch 塊 一些開(kāi)發(fā)者聽(tīng)到try/catch塊這個(gè)名字后，就會(huì )想當然的以為所有的try塊必須要有與之匹配的catch塊。 C++程序員尤其是會(huì )這樣想，因為在C++中不存在finally塊的概念，而且try塊存在的唯一理由只不過(guò)是為了與catch塊相配對。 增加不必要的catch塊的代碼就象下面的樣子，捕獲到的異常又立即被拋出： // Nifty code here }catch(Exception e){ throw e; }finally{ // Cleanup code here } 不必要的catch塊被刪除后，上面的代碼就縮短為： try{ // Nifty code here }finally{ // Cleanup code here } 常見(jiàn)錯誤11#；沒(méi)有正確實(shí)現equals，hashCode，或者clone 等方法 方法equals，hashCode，和clone 由java.lang.Object提供的缺省實(shí)現是正確的。不幸地是，這些缺省實(shí)現在大部分時(shí)候毫無(wú)用處，因此許多類(lèi)覆蓋其中的若干個(gè)方法以提供更有用的功能。但是，問(wèn)題又來(lái)了，當繼承一個(gè)覆蓋了若干個(gè)這些方法的父類(lèi)的時(shí)候，子類(lèi)通常也需要覆蓋這些方法。在進(jìn)行代碼審查時(shí)，應該確保如果父類(lèi)實(shí)現了equals，hashCode，或者clone等方法，那么子類(lèi)也必須正確。正確的實(shí)現equals，hashCode，和clone需要一些技巧。 小結 我在代碼審查的時(shí)候至少遇到過(guò)一次這些錯誤，我自己也犯過(guò)其中的幾個(gè)錯誤。好消息是只要你知道你在找什么錯誤，那么代碼審查就很容易管理，錯誤也很容易被發(fā)現和修改。即便你找不到時(shí)間來(lái)進(jìn)行正規的代碼審查，以自審的方式把這些錯誤從你的代碼中根除會(huì )大大節省你的調試時(shí)間?；〞r(shí)間在代碼審查上是值得的。 public void wrongStore (int[][] data)
Integer i = new Integer (400); int x = 4; public class Example{ public void increment (){ public synchronized void increment (){ OutputStream os = null; OutputStream os = null; try{
聯(lián)系客服
