[Refactoring] Demo video

原本這程式會分別出兩張報表，新的需求是要併成一張，

看到程式的第一個念頭就是「重構」，

請記住！「重構」不是「重寫」，

我們是懷著讓原本程式更容易閱讀的心情再做「重構」的。

影片不是很清楚，程式不是很容易閱讀，純粹紀錄「重構」的感覺，

如果你也進行過「重構」工作，我想你會懂我的。

[Refactoring] 愛他就請在最接近第一次使用他的地方宣告他(變數)

變數宣告是不是一定要在函數的開頭？

在 C 的年代，問這種問題應該會被恥笑到抬不起頭，
沒有人去挑戰，因為 Compiler 就告訴你這樣不行(其實是因為 Compiler 這樣比較好做)，
而現在， C 的輝皇年代過去了，後繼的程式語言取消了這個限制，
於是我們再來討論一下這個問題：變數宣告是不是一定要在函數的開頭？

就我認為，沒這個必要，因為他一點好處都沒有，
反而會帶來不必要的困擾，常常會看到那種宣告完了，
就再也不用他的變數，因為你並不是為了用他而宣告他，
而是覺得等一下會用，但寫了幾行程式碼之後，就把他忘得一乾二淨了，
但這並不是最惹人厭煩的，

試從以下這種角度思考：
假設函數有 20 行程式碼，我在第一行便宣告了一個變數，
這代表接下來的 19 行程式碼都看得見他，
即使他們一點關系都沒有，如果這中間又做了什麼處理，
要知道這個變數最終的結果，或者這變數會影響什麼，
你都無法忽略這 19 行程式碼，
那如果是在第 10 行宣告呢？
這表示前 10 行程式和這變數一點關系都沒有，
你只要關心剩下來的 10 行程式，多開心ㄚ。

也許你會說，那全域變數怎麼辦，但，別忘了，我這裡說的是函數中的區域變數，
既然談到了全域變數，那就來探討一下全域變數，
就我的觀點，是能不用就不用，從上面的角度思考，
假設程式有 1000 行，我在第一行便宣告了一個變數，
這表示接下來的 999 行程式碼都可以看到他，
方便是方便，但這也表示如果這個變數有問題，
你得看完這 999 行程式碼，也許你又會說，
可我只有在一兩個地方用到他，所以其實我只要看那一兩個地方，
除非你的程式很小，一般情況，你會很快的忘了你只有在一兩個地方用過他，
更甚者在多人開發的情況下，沒人會知道只在一兩個地方用他，
甚至有人還會參一腳，把他拿去用，而這些情況都會讓你的程式耦合度變高，

所以，我主張，變數的能見範圍，最好是夠用就好，
像 Code Complete 書上說的，在最接近第一次使用他的地方宣告他即可，
別再偏執的把區域變數都集中在一塊宣告了。

PS. 全域變數，如果真的要用，那還是集中在一起比較好，
因為你可能會想知道他們各自的初始值是什麼，
但，聽我個忠告，能不用就別再用他會對你比較好。

[Refactoring] 去除讓人困惑的巢狀判斷

if 很好用，但遇到一層又一層，一層又一層巢狀 if 的時候常常搞得人昏頭轉向，

今天剛好看到一個很經典的案例，可以做重構案例的分享。

首先重構的第一個步驟，先瞭解原本程式在做什麼。(因為我們並不是要重寫，只是要讓程式以更優雅的姿態呈現)，而這也是最困難的一個部分，因為一個需要重構的程式，通常都是因為不容易閱讀，才會需要重構。

所看一下程式的主要目的是什麼：

畫面上有 5 個欄位，分別是開始日期、開始日期的時間、結束日期、結束日期的時間、總時數； checkTimeFormat 被呼叫到的時機是，當相關的欄位 onchange 時就觸發。

function checkTimeFormat() {

    var startDate = new Date(ctrls.idtpCouseOpen_DATE_txtPreBox_txtDate.val());

    var endDate = new Date(ctrls.idtpCouseOpen_DATE_txtPostBox_txtDate.val());

    var startTime = ctrls.itxtCouseOpen_TIME_txtPreBox.val();

    var endTime = ctrls.itxtCouseOpen_TIME_txtPostBox.val();

    if ((!isNaN(startDate)) & (!isNaN(endDate))) {

        if ((!isNaN(startTime)) & (!isNaN(endTime))) {

            if (((parseInt(startTime) < 2359) & (parseInt(endTime)) < 2359)) {

                if ((parseInt(startTime)) < (parseInt(endTime))) {

                    caculateTTLHR(startDate, endDate, startTime, endTime);

                } else {

                    alert("開始時間大於結束時間");

                    ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

                    ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

                    ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                }

            }

            else {

                if (parseInt(startTime) > 2359) {

                    alert("時間輸入錯誤，請重新輸入！");

                    ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

                    ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                }

                if (parseInt(endTime) > 2359) {

                    alert("時間輸入錯誤，請重新輸入！");

                    ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

                    ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                }

            }

        }

        else {

            if (isNaN(startTime)) {

                alert("時間輸入錯誤，請重新輸入！");

                ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

            }

            if (isNaN(endTime)) {

                alert("時間輸入錯誤，請重新輸入！");

                ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

            }

        }

    }

}

以下就程式判斷的邏輯做分析

1. 如果日期欄位轉成日期物件後不是非數值內容，才處理，

    否則什麼事都不做。

2. 如果時間欄位都有輸入非數值內容時，檢查日期、時間輸入是否合邏輯

    否則分別判斷哪個時間欄位非數值，並提示使用者時間欄位輸入錯誤。

3. 承 2 ，如果輸入的時間 < 2359 才判斷日期和時間的起迄關系(起不可大於迄)

    否則分別判斷哪個時間欄位大於 2359 並提示使用者時間欄位輸入錯誤。

4. 承 3 ，如果起時間小於迄時間才計算時數，

    否則提示使用者起迄時間輸入有誤。

    (因為需求的關系，只針對時間的起迄判斷，也就是說日期和時間並沒有關系)

>>> 考慮用 Replace Netsted Conditional with Guard Clauses

接下來就是思考如何重構，

從上面程式來看，最深有 4 層 if ，而最終正確結果只有一個(事情的真相只有一個，唯一看透了真相是一個外表看似小孩，智慧卻過於常人的名偵探柯南 XD)，也就是最深那層的判斷，

其他的分支，都是錯誤的情況，且錯誤發生後續都不用再做額外的處理，

這是很典型可以用 Guard Clauses 處理的結構。

1. 我們把第一個 if 拿到最前面 ! 拿掉 & 改成 ||，成立便 return，馬上去掉一層，就語意層面來看也清楚多了（如果起時間不是數值 或 迄時間不是數值 就 離開不處理）。

if ((isNaN(startDate)) || (isNaN(endDate))) return; 

2. 把第二層的 if 中，else 的部分拿到最前面，於是我們又少了一層 if ，而且少了一個 if 判斷。

            if (isNaN(startTime)) {

                alert("時間輸入錯誤，請重新輸入！");

                ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

                return;

            }

            if (isNaN(endTime)) {

                alert("時間輸入錯誤，請重新輸入！");

                ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

                return;

            }

3. 把第三層的 if 中， else 的部分拿到最前面，結果同 2

                if (parseInt(startTime) > 2359) {

                    alert("時間輸入錯誤，請重新輸入！");

                    ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

                    ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");
                    return;

                }

                if (parseInt(endTime) > 2359) {

                    alert("時間輸入錯誤，請重新輸入！");

                    ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

                    ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

                    return;

                }

於是我們就初步完成了 Replace Netsted Conditional with Guard Clauses
這樣程式是不是更容易閱讀了呢？(雖然還有重構的空間，不過就先這樣囉)

function checkTimeFormat() {

    var startDate = new Date(ctrls.idtpCouseOpen_DATE_txtPreBox_txtDate.val());

    var endDate = new Date(ctrls.idtpCouseOpen_DATE_txtPostBox_txtDate.val());

    var startTime = ctrls.itxtCouseOpen_TIME_txtPreBox.val();

    var endTime = ctrls.itxtCouseOpen_TIME_txtPostBox.val();

    if ((isNaN(startDate)) || (isNaN(endDate))) return;

    if (isNaN(startTime)) {

        alert("時間輸入錯誤，請重新輸入！");

        ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

        ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

        return;

    }

    if (isNaN(endTime)) {

        alert("時間輸入錯誤，請重新輸入！");

        ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

        ctrls.itxtCouseOpen_TIME_txtPostBox.val("");
        return;

    }

    if (parseInt(startTime) > 2359) {

        alert("時間輸入錯誤，請重新輸入！");

        ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

        ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");
        return;

    }

    if (parseInt(endTime) > 2359) {

        alert("時間輸入錯誤，請重新輸入！");

        ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

        ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");
        return;

    }

    if ((parseInt(startTime)) < (parseInt(endTime))) {

        caculateTTLHR(startDate, endDate, startTime, endTime);

    } else {

        alert("開始時間大於結束時間");

        ctrls.itxtCouseOpen_TIME_txtPreBox.val("");

        ctrls.itxtCouseOpen_TIME_txtPostBox.val("");

        ctrls.txtTRAIN_CLASS_TOTALHR_EditText.val("");

    }

}

上面重構完之後，經測試發現有問題，

輸入起時間還沒輸迄時間的時候，會說開始時間大於結束時間，

原因在於如果時間欄位為 "" 空字串時， isNaN 會是 false 但是 parseInt 後會變成 NaN，

而在步驟 3 去掉這層 if 的時候，

本以為不是 parseInt > 2359 就是 parseInt < 2359，

結果出現了第三種可能，使得原來的程式不會去執行到開始時間小於結束時間的判斷。

而這個狀況雖然使得原來執行的結果是正確，但程式表達出來的語意卻不是這個樣子。

所以這邊要修正這個 Bug 就在判斷開始時間大於結束時間之前再加上

if (endTime=="" ||startTime=="") return;

最後，這支程式還有一個問題，就是 parseInt 方法 應該要傳入兩個參數 ~~ 才不會有 '08' 的問題 ~~