代码审查是消灭Bug最重要的办法之一，这些审查在大少数时分都特别奏效。由于代码审查本身所针对的对象，就是俯瞰整个代码在测试过程中的成绩和Bug。并且，代码审查抵消弭一些特别细节的错误大有裨益，尤其是那些可以容易在阅读代码的时分发现的错误，这些错误往往不容易通过机器上的测试辨认出来。本文就常见的Java代码中容易出现的成绩提出一些建立性建议，以便您在审查代码的过程中留意到这些常见的细节性错误。
通常给别人的任务挑错要比找本人的错容易些。别样视角的存在也解释了为什么作者需求编辑，而运发动需求教练的原因。不仅不该当拒绝别人的批判，我们应该欢迎别人来发现并指出我们的编程任务中的缺乏之处，我们会收获颇丰的。
正规的代码审查(code inspection)是进步代码质量的最强大的技术之一，代码审查—由同事们寻觅代码中的错误—所发现的错误与在测试中所发现的错误不同，因此两者的关系是互补的，而非竞争的。
如果审查者可以有认识地寻觅特定的错误，而不是靠漫无目的的浏览代码来发现错误，那么代码审查的效果会事半功倍。在这篇文章中，我列出了11个Java编程中常见的错误。你可以把这些错误添加到你的代码审查的反省列表(checklist)中，这样在经过代码审查后，你可以确信你的代码中不再存在这类错误了。
一、常见错误1# ：多次拷贝字符串
测试所不能发现的一个错误是生成不可变(immutable)对象的多份拷贝。不可变对象是不可改动的，因此不需求拷贝它。最常用的不可变对象是String。
如果你必须改动一个String对象的内容，你应该使用StringBuffer。下面的代码会正常任务：
String s = new String ("Text here");
但是，这段代码功用差，而且没有必要这么复杂。你还可以用以下的方式来重写下面的代码：String temp = "Text here"; String s = new String (temp);
但是这段代码包含额外的String，并非完全必要。更好的代码为：String s = "Text here";
二、常见错误2#： 没有克隆(clone)返回的对象
封装(encapsulation)是面向对象编程的重要概念。不幸的是，Java为不小心打破封装提供了方便——Java允许返回私有数据的引用(reference)。下面的代码提醒了这一点：import java.awt.Dimension; /***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; } }
Example类保证了它所存储的height和width值永远非负数，试图使用setValues()办法来设置负值会触发异常。不幸的是，由于getValues()返回d的引用，而不是d的拷贝，你可以编写如下的破坏性代码：Example ex = new Example(); Dimension d = ex.getValues(); d.height = -5; d.width = -10;
如今，Example对象拥有负值了!如果getValues() 的调用者永远也不设置返回的Dimension对象的width 和height值，那么仅凭测试是不能够检测到这类的错误。
不幸的是，随着时间的推移，客户代码能够会改动返回的Dimension对象的值，这个时分，追随错误的根源是件单调且费时的事情，尤其是在多线程环境中。
更好的方式是让getValues()返回拷贝：public synchronized Dimension getValues(){ return new Dimension (d.x, d.y); }
如今，Example对象的外部形态就安全了。调用者可以根据需求改动它所得到的拷贝的形态，但是要修改Example对象的外部形态，必须通过setValues()才可以。
三、常见错误3#：不必要的克隆
我们如今知道了get办法应该返回外部数据对象的拷贝，而不是引用。但是，事情没有绝对：/*** Example class.The value should never * be negative.*/ 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()); } }
这段代码是安全的，但是就象在错误1#那样，又作了多余的任务。Integer对象，就象String对象那样，一旦被创立就是不可变的。因此，返回外部Integer对象，而不是它的拷贝，也是安全的。
办法getValue()应该被写为：public synchronized Integer getValue(){ // ’i’ is immutable, so it is safe to return it instead of a copy. return i; }
Java顺序比C++顺序包含更多的不可变对象。JDK 所提供的若干不可变类包括：
Boolean
Byte
Character
Class
Double
Float
Integer
Long
Short
String
大部分的Exception的子类
四、常见错误4# ：自编代码来拷贝数组
Java允许你克隆数组，但是开发者通常会错误地编写如下的代码，成绩在于如下的循环用三行做的事情，如果采用Object的clone办法用一行就可以完成：public class Example{ 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 = data; } }
这段代码是正确的，但却不必要地复杂。saveCopy()的一个更好的完成是：void saveCopy (int[] data){ try{ copy = (int[])data.clone(); }catch (CloneNotSupportedException e){ // Can’t get here. } }
如果你经常克隆数组，编写如下的一个工具办法会是个好主意：static int[] cloneArray (int[] data){ try{ return(int[])data.clone(); }catch(CloneNotSupportedException e){ // Can’t get here. } }
这样的话，我们的saveCopy看起来就更简洁了：void saveCopy (int[] data){ copy = cloneArray ( data); }
五、常见错误5#：拷贝错误的数据
有时分顺序员知道必须返回一个拷贝，但是却不小心拷贝了错误的数据。由于仅仅做了部分的数据拷贝任务，下面的代码与顺序员的意图有偏差：import java.awt.Dimension; /*** 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(); } }
这儿的成绩在于getValues()办法仅仅克隆了数组，而没有克隆数组中包含的Dimension对象，因此，虽然调用者无法改动外部的数组使其元素指向不同的Dimension对象，但是调用者却可以改动外部的数组元素(也就是Dimension对象)的内容。办法getValues()的更好版本为：public synchronized Dimension[] getValues() throws CloneNotSupportedException{ Dimension[] copy = (Dimension[])d.clone(); for (int i = 0; i copy.length; ++i){ // NOTE: Dimension isn’t cloneable. if (d != null) copy = new Dimension (d.height, d.width); } return copy; }
在克隆原子类型数据的多维数组的时分，也会犯类似的错误。原子类型包括int,float等。复杂的克隆int型的一维数组是正确的，如下所示：public void store (int[] data) throws CloneNotSupportedException{ this.data = (int[])data.clone(); // OK }
拷贝int型的二维数组更复杂些。Java没有int型的二维数组，因此一个int型的二维数组实践上是一个这样的一维数组：它的类型为int[]。复杂的克隆int[][]型的数组会犯与下面例子中getValues()办法第一版本同样的错误，因此应该防止这么做。下面的例子演示了在克隆int型二维数组时错误的和正确的做法：public void wrongStore (int[][] data) 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 != null) this.data = (int[])data.clone(); } }
六、常见错误6#：反省new 操作的后果是否为null
Java编程老手有时分会反省new操作的后果是否为null。能够的反省代码为：Integer i = new Integer (400); if (i == null) throw new NullPointerException();
反省当然没什么错误，但却不必要，if和throw这两行代码完全是浪费，他们的唯一功用是让整个顺序更臃肿，运转更慢。
C/C++顺序员在开端写java顺序的时分经常会这么做，这是由于反省C中malloc()的返回后果是必要的，不这样做就能够发生错误。反省C++中new操作的后果能够是一个好的编程行为，这依赖于异常是否被使能(许多编译器允许异常被制止，在这种情况下new操作失败就会返回null)。在java 中，new 操作不允许返回null，如果真的返回null，很能够是虚拟机崩溃了，这时分即便反省返回后果也杯水车薪。
七、常见错误7#：用== 替代.equals
在Java中，有两种方式反省两个数据是否相等：通过使用==操作符，或者使用所有对象都完成的.equals办法。原子类型(int, flosat, char 等)不是对象，因此他们只能使用==操作符，如下所示：int x = 4; int y = 5; if (x == y) System.out.println ("Hi"); // This ’if’ test won’t compile. if (x.equals (y)) System.out.println ("Hi");
对象更复杂些，==操作符反省两个引用是否指向同一个对象，而equals办法则完成更专门的相等性反省。
更显得混乱的是由java.lang.Object 所提供的缺省的equals办法的完成使用==来复杂的判别被比较的两个对象是否为同一个。
许多类覆盖了缺省的equals办法以便更有用些，比方String类，它的equals办法反省两个String对象是否包含同样的字符串，而Integer的equals办法反省所包含的int值是否相等。
大部分时分，在反省两个对象是否相等的时分你应该使用equals办法，而对于原子类型的数据，你用该使用==操作符。
八、常见错误8#： 混淆原子操作和非原子操作
Java保证读和写32位数或者更小的值是原子操作，也就是说可以在一步完成，因此不能够被打断，因此这样的读和写不需求同步。以下的代码是线程安全(thread safe)的：public class Example{ private int value; // More code here... public void set (int x){ // NOTE: No synchronized keyword this.value = x; } }
不过，这个保证仅限于读和写，下面的代码不是线程安全的：public void increment (){ // 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; }
在测试的时分，你能够不会捕获到这个错误。首先，测试与线程有关的错误是很难的，而且很耗时间。其次，在有些机器上，这些代码能够会被翻译成一条指令，因此任务正常，只有当在其它的虚拟机上测试的时分这个错误才能够显现。因此最好在开端的时分就正确地同步代码：public synchronized void increment (){ ++this.value; }
九、常见错误9#：在catch 块中作清除任务
一段在catch块中作清除任务的代码如下所示：OutputStream os = null; try{ os = new OutputStream (); // Do something with os here. os.close(); }catch (Exception e){ if (os != null) os.close(); }
尽管这段代码在几个方面都是有成绩的，但是在测试中很容易漏掉这个错误。下面列出了这段代码所存在的三个成绩：
1. 语句os.close()在两处出现，多此一举，而且会带来维护方面的麻烦。
2. 下面的代码仅仅处理了Exception，而没有触及到Error。但是当try块运转出现了Error，流也应该被关闭。
3. close()能够会抛出异常。
下面代码的一个更优版本为：OutputStream os = null; try{ os = new OutputStream (); // Do something with os here. }finally{ if (os != null) os.close(); }
这个版本消弭了下面所提到的两个成绩：代码不再重复，Error也可以被正确处理了。但是没有好的办法来处理第三个成绩，也许最好的办法是把close()语句独自放在一个try/catch块中。
十、常见错误10#： 增加不必要的catch 块
一些开发者听到try/catch块这个名字后，就会想当然的以为所有的try块必需要有与之匹配的catch块。
C++顺序员尤其是会这样想，由于在C++中不存在finally块的概念，而且try块存在的唯一理由只不过是为了与catch块相配对。
增加不必要的catch块的代码就象下面的样子，捕获到的异常又立即被抛出：try{ // Nifty code here }catch(Exception e){ throw e; }finally{ // Cleanup code here }
不必要的catch块被删除后，下面的代码就延长为：try{ // Nifty code here }finally{ // Cleanup code here }
十一、常见错误11#;没有正确完成equals，hashCode，或者clone 等办法
办法equals，hashCode，和clone 由java.lang.Object提供的缺省完成是正确的。不幸地是，这些缺省实如今大部分时分毫无用途，因此许多类覆盖其中的若干个办法以提供更有用的功用。但是，成绩又来了，当继承一个覆盖了若干个这些办法的父类的时分，子类通常也需求覆盖这些办法。在进行代码审查时，应该确保如果父类完成了equals，hashCode，或者clone等办法，那么子类也必须正确。正确的完成equals，hashCode，和clone需求一些技巧。
小结
我在代码审查的时分至多遇到过一次这些错误，我本人也犯过其中的几个错误。好消息是只需你知道你在找什么错误，那么代码审查就很容易管理，错误也很容易被发现和修改。即便你找不到时间来进行正规的代码审查，以自审的方式把这些错误从你的代码中根除会大小节省你的调试时间。花时间在代码审查上是值得的。文章由姿美堂整理，收集辛苦，希望能保留出处，谢谢斑竹大哥。
