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

2019-08-27

1317
0
TypeScript

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

大概也是因為來寫網頁後很少再有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 = ["&nbsp;", "填寫基本資料", "經費編列", "填寫計畫內容", "填寫實施步驟與進度", "附件上傳", "送出確認"];
var stepListValue = [
     ["&nbsp;", "填寫中", "未填寫", "未填寫", "未填寫", "　", "　"],
     ["&nbsp;", "已填寫", "填寫中", "未填寫", "未填寫", "　", "　"],
     ["&nbsp;", "已填寫", "已填寫", "已填寫", "已填寫", "　", "　"]
];
//#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;
    };
}

回首頁

屎弟夫

程式紀錄

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

標籤雲

系列文章