重構程式碼，就是找出不良的寫法並不斷改進的行為，每一步如果都有測試把關，就能更安全的進行下一步，而不用自己找出問題點。當然，這一切的前提是測試必須簡單到只有一個可能的影響點，這樣才能快速的找出出問題的部分
我們要進行的第一個重構是對控制域的重構
>> 當屬性應當被保護時(不被外界存取)，我們就應該給予私有等級的權限
將Account的ID password都改為private，在將balance也改為private的時候，我們遇上了問題，我們不能直接取用Account實體的balance了，為了讓操作最小化，所以我們給予一個getter，就像這樣
public double balance() {
return balance;
}
所以我們就能像下面這樣存取
double balance = ac.balance();
接著執行測試，失敗!!!
因為ID還有password都有同樣的問題，所以都給予getter方法
現在測試通過，運行正常，所以我們要找尋下一個部分進行改善
在程式設計中，有一個原則叫Don't repeat yourself，意思是不要寫雖有不同卻類似的程式碼
在Account類別身上，我們找到了這個問題，其建構式如下
Account(String ID, String password) {
this.ID = ID;
this.password = password;
this.balance = 0;
}
Account(String ID, String password, double balance) {
this.ID = ID;
this.password = password;
this.balance = balance;
}
唯一的差別是，當沒有指定存款金額時，將其值設為0，那麼事實上我們可以這樣做
Account(String ID, String password) {
this(ID, password, 0);
}
其中一個建構式委託另一個建構式來處理即可(並且附上註解，說明原由)
ps. 有些語言對此問題的解法是讓你可以指定預設值，實現上更優雅簡單
因為Account目前是獨立的，所以將它移到專屬檔案存放
而它的測試也不再需要透過User實例測試它，所以讓我們改掉這個情形
public void depositTest() {
Account ac = new Account("1", "1");
ac.userDeposit(10000);
Assert.assertEquals(10000, ac.balance(), 0.0000001);
}
這是其中一個，其他的測試函式也照做就行
那麼，我們謹遵DRY原則，所以我把ac變成了一個靜態(static)物件，好讓下面的程式不必重複宣告
將各個測試函數中的ac移除之後，很不幸的是只有第一個測試程式成功了，道理很簡單，因為現在ac的副作用隨著共用同一個實例而被展現出來
我們改一改之後，程式可以運作了，然而它卻依賴著測試程式的執行順序!!!
一旦我們試圖個別執行測試，它們就不能正常運作
所以我們只好依靠JUnit提供的功能處理這件事
private Account ac;
@Before
public void setup() {
ac = new Account("1", "1", 50000);
}
@After
public void next() {
ac = null;
}
非常直觀的是，Before在每個測試執行前被執行，After恰恰相反
現在測試程式既能夠獨立執行也能一次執行全部
突然，有使用者抱怨------他明明還有50000元，為何提領50000元卻得到餘額不足的結果呢?
我們深入追查才發現，原來我們出了個烏龍
if (amountOfWithdraw < balance)
這裡應該是等於時也可以，所以我們馬上修正它，並在測試案例中加上了這個特殊情形
@Test
public void withdrawSomeTest() {
double excepted = 0;
try {
ac.userWithdraw(50000);
} catch (BalanceException e) {
excepted = 50000;
} finally {
Assert.assertEquals(excepted, ac.balance(), 0.0000001);
}
}
這下問題解決了
再來我把BalanceException換成了比較貼切的OutOfMoneyException
Account.XXX的amountOfXXX都改成amount(由於他們所處的位置變數不多，採簡單的命名就行了)
接下來都是名稱變換，並不重要
真正的問題在，從使用案例中，我認為User作為實例的意義已經不存在了，它現在只是純粹的使用者介面部分，那麼我們就開始著手改變其架構吧!
public class DataBase {
List<Account> Accounts = new ArrayList<>();
DataBase() {
Accounts.add(new Account("1", "1", 50000));
}
ResultAccount find(String userID, String userPassword) {
ResultAccount answer = new ResultAccount();
for (Account ac : Accounts) {
if (ac.ID().equals(userID) && ac.password().equals(userPassword)) {
answer.location = Accounts.indexOf(ac);
answer.found = true;
return answer;
}
}
answer.found = false;
return answer;
}
}
先是DataBase改為存放帳戶訊息，接著將測試中的存取方式全部改掉，這時候，測試的威力就出現了，這次沒有測試失敗，因此我們可以相信這次改動應該是完全正確的!
我們不必做猜測
接著另Accounts成為private，因為我們可不希望帳戶資料可以在外面被隨意修改，對吧!
更重要的是，我們可以實現檢查邏輯，避免重複的ID存在，下面是我們提供的新介面方法，以因應適才所言的部分
public void add(Account ac) {
Accounts.add(ac);
}
這個方法還沒辦法檢查是否重複增加成員，所以我們改成下面那樣
public void add(Account ac) {
if(find(ac.ID())) {
return;
}
Accounts.add(ac);
}
boolean find(String ID) {
for (Account ac : Accounts) {
if (ac.ID().equals(ID)) {
return true;
}
}
return false;
}
現在我們可以看出是否已經有這個ID了，重複的帳號將不會被加入
我們依樣畫葫蘆的試圖重現get
public Account get(int index) {
return Accounts.get(index);
}
然而這次卻失敗了，我們只得到一個NullPointerException，為什麼呢?就留給你自己去探查
更重要的是，無論對這段程式如何修改都沒有意義，因為架構已全然不同，我們的User已經失去跟Account的直接關聯
而且在這裡，我們要調用的是User實例，所以應該把get移除，並且改成
for (Method m : user.getClass().getDeclaredMethods()) {
IfChoose c = m.getAnnotation(IfChoose.class);
if (choose == c.value()) {
double balance = (double) m.invoke(user);
System.out.println("剩餘 " + balance + "元");
break;
}
}
這樣調用，user宣告成Main的static物件
然而，現在的User身上不應該出現Account實體，所以我們把它移除，既然已經沒有field了，那麼採用預設建構式就可以了
原本的find(String, String)複雜無比，讓我們採用更簡單的方式
int find(String userID, String userPassword) {
int i = -1;
for (Account ac : Accounts) {
if (ac.ID().equals(userID) && ac.password().equals(userPassword)) {
i = Accounts.indexOf(ac);
return i;
}
}
return i;
}
而使用方式自然要跟進
int index = db.find(userID, userPassword);
if (index != -1) {
// ...
}
可見我當時做了過度設計，為不存在的需求設計了多餘的類別
而測試在修改之後也成功通過了
我們根據新架構提出了新的方式
double depositFrom(int i, int depositAmount) {
return Accounts.get(i).userDeposit(depositAmount);
}
double withdrawFrom(int i, int depositAmount) throws OutOfMoneyException {
return Accounts.get(i).userWithdraw(depositAmount);
}
double getBalanceFrom(int i) {
return Accounts.get(i).balance();
}
因為在find的時候，我們知道了究竟是在使用哪一個帳號，但是我們沒辦法透過User實例直接調用了
@IfChoose(value = 1)
double deposit(int index) {
System.out.println("請輸入存款金額:");
int depositAmount = Main.sc.nextInt();
return Main.db.depositFrom(index, depositAmount);
}
於是我們只能將User.deposit()改成這樣，不過利用反射調用deposit的部分，得加上這個新參數
double balance = (double) m.invoke(user, index);
其他部分也如此改造就行了
在成功運作之後我們要想怎麼改進，顯然，我們這樣搞不但重複多，現在登入哪一個帳戶我們都不大清楚，而傳遞的風險還有可能被不知情的程式員改動這個值，雖然我們可以用final來確保值不被改變，然而我們並不能確保這個放置在外的程式不會被改變，因此最保險的方式是封裝它
boolean find(String userID, String userPassword) {
for (Account ac : Accounts) {
if (ac.ID().equals(userID) && ac.password().equals(userPassword)) {
now = Accounts.indexOf(ac);
return true;
}
}
return false;
}
double depositFrom(int depositAmount) {
int i = now;
now = -1;
return Accounts.get(i).userDeposit(depositAmount);
}
double withdrawFrom(int depositAmount) throws OutOfMoneyException {
int i = now;
now = -1;
return Accounts.get(i).userWithdraw(depositAmount);
}
double getBalanceFrom() {
int i = now;
now = -1;
return Accounts.get(i).balance();
}
將他們改成這樣，測試對應上，測試通過
接著改變調用的方式，然後User的兩個方法接收index的意義已不存在，所以移除
接著增加測試
@Test
public void getBalance() {
boolean exist = db.find("1", "1");
double balance = db.getBalanceFrom();
Assert.assertEquals(50000, balance, 0.0000001);
}
這讓我們知道now是不是真的指向我們預期的那個帳戶
其他的方法也一樣加上測試
接著執行main，看起來跟我們想的一樣
這篇就先在這裡結束
下一篇我們新增一些功能，See you next time
沒有留言:
張貼留言