Java敏捷開發技巧之消除代碼異味

異味這個詞，可能有點抽象，我們先看一下下面的例子。

這是一個CAD系統。現在，它已經可以畫三種形狀了：線條，長方形和圓。先認真的看一下下面的代碼：

class Shape {
final static int TYPELINE = 0;
final static int TYPERECTANGLE = 1;
final static int TYPECIRCLE = 2;
int shapeType;
//線條的開始點
//長方形左下角的點
//圓心
Point p1;
//線條的結束點
//長方形的右上角的點
//如果是圓的話,這個屬性不用
Point p2;
int radius;
}
class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
switch (shapes[i].getType()) {
case Shape.TYPELINE:　
graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());
break;
case Shape.TYPERECTANGLE:
//畫四條邊
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
break;
case Shape.TYPECIRCLE:
graphics.drawCircle(shapes[i].getP1(), shapes[i].getRadius());
break;
}
}
}
}

代碼都是一直在改變的，而這也是上面的代碼會碰到的一個問題。

現在我們有一個問題：如果我們需要支持更多的形狀(比如三角形)，那麼肯定要改動Shape這個類，CADApp裡面的drawShapes這個方法也要改。

好，改為如下的樣子：

class Shape {
final static int TYPELINE = 0;
final static int TYPERECTANGLE = 1;
final static int TYPECIRCLE = 2;
final static int TYPETRIANGLE = 3;
int shapeType;
Point p1;
Point p2;
//三角形的第三個點.
Point p3;
int radius;
}
class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
switch (shapes[i].getType()) {
case Shape.TYPELINE:
graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());
break;
case Shape.TYPERECTANGLE:
//畫四條邊.
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
break;
case Shape.TYPECIRCLE:
graphics.drawCircle(shapes[i].getP1(), shapes[i].getRadius());
break;
case Shape.TYPETRIANGLE:
graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());
graphics.drawLine(shapes[i].getP2(), shapes[i].getP3());
graphics.drawLine(shapes[i].getP3(), shapes[i].getP1());
break;
}
}
}
}

如果以後要支持更多的形狀，這些類又要改動……，這可不是什麼好事情！

理想情況下，我們希望當一個類，一個方法或其他的代碼設計完以後，就不用再做修改了。它們應該穩定到不用修改就可以重用。

現在的情況恰好相反！

每當我們增加新的形狀，都得修改Shape這個類，跟CADApp裡面的drawShapes方法。

怎麼讓代碼穩定（也就是無需修改）？這個問題是個好問題！不過老規矩，先不說，我們以行動回答。

我們先看看另外一個方法： 當給你一段代碼，你怎麼知道它是穩定的？

怎麼判斷代碼的穩定性？

要判斷代碼的穩定性，我們可能會這樣來判定：先假設一些具體的情況或者需求變動了，然後來看一看，要滿足這些新的需求，代碼是否需要被修改？

可惜，這也是一件很麻煩的事，因為有那麼多的可能性！我們怎麼知道哪個可能性要考慮，哪些不用考慮？

有個更簡單的方法，如果發現說，我們已經第三次修改這些代碼了，那我們就認定這些代碼是不穩定的。這個方法很“懶惰”，而且“被動”！我們被傷到了，才開始處理狀況。不過至少這種方法還是一個很有效的方法。

此外，還有一個簡單，而且“主動”的方法：如果這段代碼是不穩定或者有一些潛在問題的，那麼代碼往往會包含一些明顯的痕跡。正如食物要腐壞之前，經常會發出一些異味一樣（當然，食物如果有異味了，再怎麼處理我們都不想吃了。但是代碼可不行。）。我們管這些痕跡叫做“代碼異味”。正如並不是所有的食物有異味都不能吃了，但大多數情況下，確實是不能吃了。並不是所有的代碼異味都是壞事，但大多數情況下，它們確實是壞事情！因此，當我們感覺出有代碼異味時，我們必須小心謹慎的檢查了。

現在，我們來看看上面例子中的代碼異味吧！

示例代碼中的代碼異味：

第一種異味：代碼用了類別代碼（type code）

class Shape {
final int TYPELINE = 0;
final int TYPERECTANGLE = 1;
final int TYPECIRCLE = 2;
int shapeType;
...　
}

這樣的異味，是一種嚴肅的警告：我們的代碼可能有許多問題。

第二種異味：Shape這個類有很多屬性有時候是不用的。例如，radius這個屬性只有在這個Shape是個圓的時候才用到：

class Shape {
...　
Point p1;
Point p2;
int radius; //有時候不用
}

第三種異味：我們想給p1,p2取個好一點的變量名都做不到，因為不同的情況下，它們有不同的含義：

class Shape {
...
Point p1; //要取作“起始點”，“左下點”，還是“圓心”？
Point p2;
}

第四種異味：drawShapes這個方法裡面，有個switch表達式。當我們用到switch（或者一大串的if-then-else-if）時，小心了。switch表達式經常是跟類別代碼（type code）同時出現的。

現在，讓我們將這個示例中的代碼異味消除吧！

消除代碼異味：怎麼去掉類別代碼（type code）

大多數情況下，要想去掉一個類別代碼，我們會為每一種類別建立一個子類，比如（當然，並不是每次要去掉一個類別代碼都要增加一個新類，我們下面的另一個例子裡面會講另一種解決方法）：

class Shape {
}
class Line extends Shape {
Point startPoint;
Point endPoint;
}
class Rectangle extends Shape {
Point lowerLeftCorner;
Point upperRightCorner;
}
class Circle extends Shape {
Point center;
int radius;
}

因為現在沒有類別代碼了，drawShapes這個方法裡面，就要用instanceof來判斷對象是哪一種形狀了。因此，我們不能用switch了，而要改用if-then-else：

class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
if (shapes[i] instanceof Line) {
Line line = (Line)shapes[i];
graphics.drawLine(line.getStartPoint(),line.getEndPoint());
} else if (shapes[i] instanceof Rectangle) {
Rectangle rect = (Rectangle)shapes[i];
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
} else if (shapes[i] instanceof Circle) {
Circle circle = (Circle)shapes[i];
graphics.drawCircle(circle.getCenter(), circle.getRadius());
}
}
}
}

因為沒有類別代碼了，現在每個類（Shape，Line，Rectangle，Circle）裡面的所有屬性就不會有時用得到，有時用不到了。現在我們也可以給它們取一些好聽點的名字了（比如在Line裡面，p1這個屬性可以改名為startPoint了）。現在四種異味只剩一種了，那就是，在drawShapes裡面還是有一大串if-then-else-if。我們下一步，就是要去掉這長長的一串。

消除代碼異味：如何去掉一大串if-then-else-if（或者switch）

經常地，為了去掉if-then-else-if或者switch，我們需要先保證在每個條件分支下的要寫的代碼是一樣的。在drawShapes這個方法裡面，我們先以一個較抽象的方法（偽碼）來寫吧！

class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
if (shapes[i] instanceof Line) {
畫線條;
} else if (shapes[i] instanceof Rectangle) {
畫長方形;
} else if (shapes[i] instanceof Circle) {
畫圓;
}
}
}
}

條件下的代碼還是不怎麼一樣，不如再抽象一點：

class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
if (shapes[i] instanceof Line) {
畫出形狀;
} else if (shapes[i] instanceof Rectangle) {
畫出形狀;
} else if (shapes[i] instanceof Circle) {
畫出形狀;
}
}
}
}

好，現在三個分支下的代碼都一樣了。我們也就不需要條件分支了：

class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
畫出形狀;
}
}
}

最後，將“畫出形狀”這個偽碼寫成代碼吧！

class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
shapes[i].draw(graphics);
}
}
}

當然，我們需要在每種Shape的類裡面提供draw這個方法：

abstract class Shape {
abstract void draw(Graphics graphics);
}
class Line extends Shape {
Point startPoint;
Point endPoint;
void draw(Graphics graphics) {
graphics.drawLine(getStartPoint(), getEndPoint());
}
}
class Rectangle extends Shape {
Point lowerLeftCorner;
Point upperRightCorner;
void draw(Graphics graphics) {
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
}
}
class Circle extends Shape {
Point center;
int radius;
void draw(Graphics graphics) {
graphics.drawCircle(getCenter(), getRadius());
}
}

將抽象類變成接口

現在，看一下Shape這個類，它本身沒有實際的方法。所以，它更應該是一個接口：

interface Shape {
void draw(Graphics graphics);
}
class Line implements Shape {
...　
}
class Rectangle implements Shape {
...　
}
class Circle implements Shape {
...　
}

改進後的代碼

改進後的代碼就像下面這樣：

interface Shape {
void draw(Graphics graphics);
}
class Line implements Shape {
Point startPoint;
Point endPoint;
void draw(Graphics graphics) {
graphics.drawLine(getStartPoint(), getEndPoint());
}
}
class Rectangle implements Shape {
Point lowerLeftCorner;
Point upperRightCorner;
void draw(Graphics graphics) {
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
graphics.drawLine(...);
}
}
class Circle implements Shape {
Point center;
int radius;
void draw(Graphics graphics) {
graphics.drawCircle(getCenter(), getRadius());
}
}
class CADApp {
void drawShapes(Graphics graphics, Shape shapes[]) {
for (int i = 0; i < shapes.length; i++) {
shapes[i].draw(graphics);
}
}
}

如果我們想要支持更多的圖形（比如：三角形），上面沒有一個類需要修改。我們只需要創建一個新的類Triangle就行了。

另一個例子

讓我們來看一下另外一個例子。在當前的系統中，有三種用戶：常規用戶，管理員和游客。

常規用戶必須每隔90天修改一次密碼（更頻繁也行）。管理員必須每30天修改一次密碼。游客就不需要修改了。

常規用戶跟管理員可以打印報表。

先看一下當前的代碼：

class UserAccount {
final static int USERTYPE_NORMAL = 0;
final static int USERTYPE_ADMIN = 1;
final static int USERTYPE_GUEST = 2;
int userType;
String id;
String name;
String password;
Date dateOfLastPasswdChange;
public boolean checkPassword(String password) {
...
}
}
class InventoryApp {
void login(UserAccount userLoggingIn, String password) {
if (userLoggingIn.checkPassword(password)) {
GregorianCalendar today = new GregorianCalendar();
GregorianCalendar expiryDate = getAccountExpiryDate(userLoggingIn);
if (today.after(expiryDate)) {
//提示用戶修改密碼
...
}
}
}
GregorianCalendar getAccountExpiryDate(UserAccount account) {
int passwordMaxAgeInDays = getPasswordMaxAgeInDays(account);
GregorianCalendar expiryDate = new GregorianCalendar();
expiryDate.setTime(account.dateOfLastPasswdChange);
expiryDate.add(Calendar.DAY_OF_MONTH, passwordMaxAgeInDays);
return expiryDate;
}
int getPasswordMaxAgeInDays(UserAccount account) {
switch (account.getType()) {
case UserAccount.USERTYPE_NORMAL:
return 90;
case UserAccount.USERTYPE_ADMIN:
return 30;
case UserAccount.USERTYPE_GUEST:
return Integer.MAX_VALUE;
}
}
void printReport(UserAccount currentUser) {
boolean canPrint;
switch (currentUser.getType()) {
case UserAccount.USERTYPE_NORMAL:
canPrint = true;
break;
case UserAccount.USERTYPE_ADMIN:
canPrint = true;
break;
case UserAccount.USERTYPE_GUEST:
canPrint = false;
}
if (!canPrint) {
throw new SecurityException("You have no right");
}
//打印報表
}
}

用一個對象代替一種類別（注意，之前是一個類代替一種類別）。

根據之前講的解決方法，要去掉類別代碼，我們只需要為每種類別創建一個子類，比如：

abstract class UserAccount {
String id;
String name;
String password;
Date dateOfLastPasswdChange;
abstract int getPasswordMaxAgeInDays();
abstract boolean canPrintReport();
}
class NormalUserAccount extends UserAccount {
int getPasswordMaxAgeInDays() {
return 90;
}
boolean canPrintReport() {
return true;
}
}
class AdminUserAccount extends UserAccount {
int getPasswordMaxAgeInDays() {
return 30;
}
boolean canPrintReport() {
return true;
}
}
class GuestUserAccount extends UserAccount {
int getPasswordMaxAgeInDays() {
return Integer.MAX_VALUE;
}
boolean canPrintReport() {
return false;
}
}

但問題是，三種子類的行為（裡面的代碼）都差不多一樣，getPasswordMaxAgeInDays這個方法就一個數值不同（30,90或者Integer.MAX_VALUE)。canPrintReport這個方法也不同在一個數值（true或false）。這三種用戶類型只需要用三個對象代替就行了，無須特別地新建三個子類了：

class UserAccount {
UserType userType;
String id;
String name;
String password;
Date dateOfLastPasswdChange;
UserType getType() {
return userType;
}
}
class UserType {
int passwordMaxAgeInDays;
boolean allowedToPrintReport;
UserType(int passwordMaxAgeInDays, boolean allowedToPrintReport) {
this.passwordMaxAgeInDays = passwordMaxAgeInDays;
this.allowedToPrintReport = allowedToPrintReport;
}
int getPasswordMaxAgeInDays() {
return passwordMaxAgeInDays;
}
boolean canPrintReport() {
return allowedToPrintReport;
}
static UserType normalUserType = new UserType(90, true);
static UserType adminUserType = new UserType(30, true);
static UserType guestUserType = new UserType(Integer.MAX_VALUE, false);
}
class InventoryApp {
void login(UserAccount userLoggingIn, String password) {
if (userLoggingIn.checkPassword(password)) {
GregorianCalendar today = new GregorianCalendar();
GregorianCalendar expiryDate = getAccountExpiryDate(userLoggingIn);
if (today.after(expiryDate)) {
//提示用戶修改密碼
...
}
}
}
GregorianCalendar getAccountExpiryDate(UserAccount account) {
int passwordMaxAgeInDays = getPasswordMaxAgeInDays(account);
GregorianCalendar expiryDate = new GregorianCalendar();
expiryDate.setTime(account.dateOfLastPasswdChange);
expiryDate.add(Calendar.DAY_OF_MONTH, passwordMaxAgeInDays);
return expiryDate;
}
int getPasswordMaxAgeInDays(UserAccount account) {
return account.getType().getPasswordMaxAgeInDays();
}
void printReport(UserAccount currentUser) {
boolean canPrint;
canPrint = currentUser.getType().canPrintReport();
if (!canPrint) {
throw new SecurityException("You have no right");
}
//打印報表.
}
}

注意到了吧！用一個對象代替類別，同樣可以移除switch或者if-then-else-if。

總結一下類別代碼的移除

要移動一些類別代碼和switch表達式，有兩種方法：

1、用基於同一父類的不同子類來代替不同的類別。

2、用一個類的不同對象來代替不同的類別。

當不同的類別具有比較多不同的行為時，用第一種方法。當這些類別的行為非常相似，或者只是差別在一些值上面的時候，用第二個方法。

普遍的代碼異味

類別代碼和switch表達式是比較普遍的代碼異味。此外，還有其他的代碼異味也很普遍。

下面是大概的異味列表：

◆代碼重復

◆太多的注釋

◆類別代碼（type code）

◆switch或者一大串if-then-else-if

◆想給一個變量，方法或者類名取個好名字時，也怎麼也取不好

◆用類似XXXUtil, XXXManager, XXXController 和其他的一些命名

◆在變量，方法或類名中使用這些單詞“And”，“Or”等等

◆一些實例中的變量有時有用，有時沒用

◆一個方法的代碼太多，或者說方法太長

◆一個類的代碼太多，或者說類太長

◆一個方法有太多參數

◆兩個類都引用了彼此（依賴於彼此）