[TypeScript] 不夠好的寫法 vs 較好的寫法,

小弟我真的還很菜,一直覺得自己會的那些東西有辦法處理大部分的需求了,就以為自己好棒棒。

大概也是因為來寫網頁後很少再有CodeReview ,今天CodeReview後拿到一堆TODO、一整個需要重新改寫的後端流程

想寫這篇也是單純想記錄下,自己修正這些錯誤、學習的過程。

也利於我自己有更深的印象、勉勵自己以後繼續努力

//TODO: 可以在StepList中用function封裝起來,避免變數暴露在全域,可能造成變數互相汙染的副作用
//const stepListClass = {
//  step1: [],
//  step2: []
//  ...etc.
//};

//enum stepClass{
//  step1 = 0,
//  step2 = 1,
//  ...etc.
//}

//#region 組成 StepList Html 內容
var stepListClass = [
     ["pstep1", "unstep1", "unstep2", "unstep3", "unstep4", "unstep5", "unstep6"],
     ["step0", "pstep2", "unstep2", "unstep3", "unstep4", "unstep5", "unstep6"], 
     ["step0", "step1", "step2", "step3", "pstep5", "step6", "unstep6"]  
];
var stepListTitle = [" ", "填寫基本資料", "經費編列", "填寫計畫內容", "填寫實施步驟與進度", "附件上傳", "送出確認"];
var stepListValue = [
     [" ", "填寫中", "未填寫", "未填寫", "未填寫", " ", " "],
     [" ", "已填寫", "填寫中", "未填寫", "未填寫", " ", " "],
     [" ", "已填寫", "已填寫", "已填寫", "已填寫", " ", " "]
];
//#endregion

1.全域變數的設立

        其實這部分我覺得我自己超白癡,在.Net我不會把變數裝在全域裡面,我也會在Ctor的時候才指定,為何會在全域這邊做,大概是因為自己當初根本還不熟TypeScript吧,在寫的時候邊寫邊查以至於根本沒發現這種錯誤...,的確在全域會造成,其他地方可能有相互作用的可能。這錯誤必須要改進。


    GetStepListHtml = function (htmlStringId: string): boolean{

        const self: StepList = this
            , $selector = $("#" + htmlStringId); //TODO: 統一用一個selecter去接,避免每次要取同一個DOM時,都全部html搜索一次

        //TODO: 加入遮罩,讀取時轉圈圈,用法為傳入要遮罩的區塊$selector
        kendo.ui.progress($selector, true);

        $.ajax({
            type: "POST",
            //TODO:有共用方法Router了,可替換成
            url: self.router.GetPath("getsteplist", "steplist"),
            //url: self.urlHelper.postUrl.getStepListUrl,
            data: {
                prjIdEncode: self.urlHelper.pageParams.prjId
            },
            dataType: "json",
            success: function (model) {
                if (model.length > 0) {
                    //取得該流程位置
                    var formStep = model[0].FormStep; //TODO: 理論上只有一筆,應該不用用List傳回來,直接回傳class的話可改寫成=> model.FormStep比較乾淨
                    //組出HTML
                    var htmlString = self.GetStepListHtmlString(formStep);                    
                    $selector.append(htmlString);   
                    kendo.ui.progress($selector, false);
                }
                else {
                    console.log("Get Table List Data fail")
                    return false;
                }                      
            },
            error: function (e) {
                console.log("fail");
                console.log(e);
                return false;
            }            
        });
        return true;
    };

2.DOM操作習慣性,一直使用重複指定

        每一次的操作DOM,都會使Jquery 重新跑一次整頁找出該Html,應用變數裝起控制項,直接帶入。

3.加入Kendo 遮罩

        這部分是屬於我自己第一次使用Kendo,對於Kendo功能的不熟悉導致,其實Kendo UI 遮罩真的不錯用,可以防止使用者重複點擊的問題,只需要傳入指定的HTML、boolean就可以控制

4.後端、前端處理資料的準確性

        這一部分在寫的時候其實很清楚就只會需要一筆,但當初只想著那我Json 取第一筆就好,但其實更好的方式不就是後端吐乾淨資料嗎? 讓後端FristOrDefault就好,我想這部分我們就從下一篇檢討.net 再來說吧

private GetStepListHtmlString = function (formStep: number): string {
        //TODO: ES6統一用let、const取代var (const最常用,表不可變動的值,去易變性)
        var html = "";
        //formStep 1 所要讀的Array為0
        var htmlClass = stepListClass[formStep - 1];//TODO:可定義一個enum用(enum)formStep去取代formStep - 1
        var htmlTitle = stepListTitle;
        var htmlValue = stepListValue[formStep - 1];
        
        html += "<ul class='step_list'>"

        //可參考https://lhammer.cn/2017/10/29/array-method/
        //取代for迴圈寫法
        for (var i = 0; i < htmlClass.length; i++) {
            html += "<li class='" + htmlClass[i] + "'>" + htmlTitle[i] + "<br />" + htmlValue[i] + "</li>"    
        }
        html += "</ul>"
        return html;
    };
}

5.宣告時別再使用var 

        如果該變數再也不會變動,就使用const,如果還會需要再變動,即使用let。

6.避免使用index -1 的方法

        其實這是我一個超爛的壞習慣,當下都會覺得先寫好就好,但其實這種解法我自己寫的當下都會覺得感覺下次來會不記得這個-1是為什麼要減,當下還很天真地覺得那我上面寫註解就好。

的確更好的方法是用enum裝起來,來取代那個-1的index

7.for迴圈的取代

        這一個其實是我第一次看到,但感覺並不陌生,有種遇到了前端LINQ的感覺,之後應該也會再寫一篇,有關於Array method操控的文章

 

之後改完的程式碼,也放在下面,如果有人有更好的寫法,我也超歡迎給我一些指教,謝謝

//TODO: 可以在StepList中用function封裝起來,避免變數暴露在全域,可能造成變數互相汙染的副作用
//const stepListClass = {
//  step1: [],
//  step2: []
//  ...etc.
//};

//enum stepClass{
//  step1 = 0,
//  step2 = 1,
//  ...etc.
//}
enum stepEnum {
    step1 = 0,
    step2 = 1,
    step3 = 2,
    step4 = 3,
    step5 = 4,
    step6 = 5
};
class StepList {    
    //分別帶入
    private observableData: kendo.data.ObservableObject;    
    private url;
    public urlHelper;
    private router: Router;
    constructor() {            
        //初始化Kendo Observe Object                        
        this.observableData = kendo.observable({});
        this.url = new Url();
        this.router = new Router();
        this.urlHelper = {
            pageParams: {                
                prjId: this.url.GetParam("prjid")
            },
            postUrl: {
                getStepListUrl: this.url.GetRootPath() + '/steplist/getsteplist'
            }
        };
    };
    GetStepListHtml = function (htmlStringId: string): boolean{

        const self: StepList = this
            , $selector = $("#" + htmlStringId); //TODO: 統一用一個selecter去接,避免每次要取同一個DOM時,都全部html搜索一次

        //TODO: 加入遮罩,讀取時轉圈圈,用法為傳入要遮罩的區塊$selector
        kendo.ui.progress($selector, true);

        $.ajax({
            type: "POST",
            //TODO:有共用方法Router了,可替換成
            url: self.router.GetPath("getsteplist", "steplist"),
            //url: self.urlHelper.postUrl.getStepListUrl,
            data: {
                prjIdEncode: self.urlHelper.pageParams.prjId
            },
            dataType: "json",
            success: function (result) {
                if (result.IsSuccess) {
                    //取得該流程位置
                    const formStep = result.Content.FormStep; //TODO: 理論上只有一筆,應該不用用List傳回來,直接回傳class的話可改寫成=> model.FormStep比較乾淨                    
                    //組出HTML
                    const htmlString = self.GetStepListHtmlString(formStep);                    
                    $selector.append(htmlString);   
                    kendo.ui.progress($selector, false);
                }
                else {
                    console.log("Get Table List Data fail")
                    return false;
                }                      
            },
            error: function (e) {
                console.log("fail");
                console.log(e);
                return false;
            }            
        });
        return true;
    };
    private GetStepListHtmlString = function (formStep: number): string {
        //#region 組成 StepList Html 內容
        const stepListClass = {
            step1: ["pstep1", "unstep1", "unstep2", "unstep3", "unstep4", "unstep5", "unstep6"],
            step2: ["step0", "pstep2", "unstep2", "unstep3", "unstep4", "unstep5", "unstep6"],
            step3: ["step0", "step1", "pstep3", "unstep3", "unstep4", "unstep5", "unstep6"],
            step4: ["step0", "step1", "step2", "pstep4", "unstep4", "unstep5", "unstep6"], 
            step5: ["step0", "step1", "step2", "step3", "pstep5", "unstep5", "unstep6"], 
            step6: ["step0", "step1", "step2", "step3", "pstep5", "step6", "unstep6"] 
        };
        const stepListTitle = ["&nbsp;", "填寫基本資料", "經費編列", "填寫計畫內容", "填寫實施步驟與進度", "附件上傳", "送出確認"];
        const stepListValue = {
            step1: ["&nbsp;", "填寫中", "未填寫", "未填寫", "未填寫", " ", " "],
            step2: ["&nbsp;", "已填寫", "填寫中", "未填寫", "未填寫", " ", " "],
            step3: ["&nbsp;", "已填寫", "已填寫", "填寫中", "未填寫", " ", " "],
            step4: ["&nbsp;", "已填寫", "已填寫", "已填寫", "已填寫", " ", " "],
            step5: ["&nbsp;", "已填寫", "已填寫", "已填寫", "已填寫", " ", " "],
            step6: ["&nbsp;", "已填寫", "已填寫", "已填寫", "已填寫", " ", " "]
        };
        //#endregion
        //TODO: ES6統一用let、const取代var (const最常用,表不可變動的值,去易變性)
        let html = "";
        //formStep 1 所要讀的Array為0
        const stepIndex = stepEnum[formStep];
        const htmlClass: Array<string> = stepListClass[stepIndex];//TODO:可定義一個enum用(enum)formStep去取代formStep - 1
        const htmlTitle: Array<string> = stepListTitle;
        const htmlValue: Array<string> = stepListValue[stepIndex];
        
        html += "<ul class='step_list'>"

        //可參考https://lhammer.cn/2017/10/29/array-method/
        //取代for迴圈寫法
        htmlClass.forEach(function (item, index) {
            html += "<li class='" + htmlClass[index] + "'>" + htmlTitle[index] + "<br />" + htmlValue[index] + "</li>"    
        });        
        html += "</ul>"
        return html;
    };
}