【JavaScript】VoidとUndefinedの話、==と===の話など

初めてのコードレビューをして頂く機会があったので、以下に（分かる範囲で）覚え書き。独学者にとっては貴重な経験であった。やっぱり一人でやっているとこういうのが分からないので、いつの間にか変な癖がつくということになりかねないですね。

・undefinedかどうか確かめたい時は、undefinedは使わない

以下のコードは、入力された値（number）が0もしくは空欄（入力なし）だった場合にアラートを表示させるためのもの。

function makeColRow() {
    let number = prompt("Enter a number between 1 to 100 to make grids.");
    if(number == false || number == undefined || number == null) { // *
        let message = alert("The number is invalid. Enter a number between 1 to 100.");
    } 

// *numberがundefinedかどうか確かめたい場合は、number === void(0)を使う

undefinedだと万が一ローカルあるいはグローバルで変数の値が上書きされていた場合、予想しない挙動になる可能性がある。
実際にはあまりなさそうだが、undefined = "example";などとすることも可能ではあるため。単純に変数が定義されているかどうかを確かめるのであれば、常にundefinedを返すvoid演算子を使うのが良い。

nullとundefinedを区別しなくてよければ、number == nullとしてもチェックは出来る。
（そういう意味では上記のコードはそもそもnumber == undefinedは必要なかった）
ただnull === undefinedではないので、100%の厳密なチェックではない。

厳密なチェックをするなら、typeof number === "undefined"でも可能。

・判定にはダブルイコールではなくトリプルイコールを

これはどこか他の記事でも読んだような気がする。
ダブルイコールだと厳密な判定が出来ないので、思わぬバグにつながる可能性が出てくる。
コードがとりあえず期待通りに動くと何となく見過ごしてしまいがちだが、トリプルしか使わない癖をつけるほうが良さそう。当然、これだとダブルイコールで動いていたものが動かないということもある（上記がまさにそのケース）ので、書き方も工夫が必要。のちほど書き直したコードを載せる。

・ifのネストは最大2つまで

ifの中にelse ifが2つも3つも入っているのは読みづらいし、テスト時も見直しづらいので極力分かりやすくすること。できればifは1つで済ませるのが理想。可読性は非常に重要ということが分かった。

これがダメな例。横長すぎて読みづらい。

function makeColRow() {
    let number = prompt("Enter a number between 1 to 100 to make grids.");
    if(number == false || number == undefined || number == null) {
        let message = alert("The number is invalid. Enter a number between 1 to 100.");
    } else if(number > 100) {
            let message = alert("The number is invalid. Enter a number between 1 to 100.");
        } else if(number <= 100) { // ifが全部で3つもある
                resetGrids();
                let square = number * number;
                let gridcounter = 0;
                while(gridcounter < square) {
                    let columns = document.createElement('div');
                    columns.className = 'columns';
                    container.appendChild(columns);
                    gridcounter++;
                }
                let clmcounter = 0;
                while(clmcounter < number) {
                    let gridtemplateclm = `grid-template-columns: repeat(` + number + `, 1fr)`;
                  let gridtemplaterow = `grid-template-rows: repeat(` + number + `, 1fr)`;
                    container.setAttribute("style", gridtemplateclm + `;` + gridtemplaterow);
                    clmcounter++;
                }
            }
    addHoverEffect();
    rounds++;
}

・結果的にこうリファクタリングした

function makeColRow() {
    let number = prompt("Enter a number between 1 to 100 to make grids.");
    number = Number(number);
    if(number === 0 || isNaN(number) === true || number > 100) {
        let message = alert("The number is invalid. Enter a number between 1 to 100.");
    } else if(number <= 100) {
            resetGrids();
            let square = number * number;
            let gridcounter = 0;
            while(gridcounter < square) {
                let columns = document.createElement('div');
                columns.className = 'columns';
                container.appendChild(columns);
                gridcounter++;
            }
            let clmcounter = 0;
            while(clmcounter < number) {
                let gridtemplateclm = `grid-template-columns: repeat(` + number + `, 1fr)`;
              let gridtemplaterow = `grid-template-rows: repeat(` + number + `, 1fr)`;
                container.setAttribute("style", gridtemplateclm + `;` + gridtemplaterow);
                clmcounter++;
            }
        }
    addHoverEffect();
    rounds++;
}

しかしこうして見ると、whileが2つ入っているのも本当は別で関数を作って呼び出すべきなのかもしれない。

・さらにリファクタリング

function makeDivs(number) {
    let square = number * number;
    let gridcounter = 0;
    while(gridcounter < square) {
        let columns = document.createElement('div');
        columns.className = 'columns';
        container.appendChild(columns);
        gridcounter++;
    }    
}

function makeGrids(number) {
    let clmcounter = 0;
    while(clmcounter < number) {
        let gridtemplateclm = `grid-template-columns: repeat(` + number + `, 1fr)`;
      let gridtemplaterow = `grid-template-rows: repeat(` + number + `, 1fr)`;
        container.setAttribute("style", gridtemplateclm + `;` + gridtemplaterow);
        clmcounter++;
    }
}

function makeColRow() {
    let number = prompt("Enter a number between 1 to 100 to make grids.");
    number = Number(number);
    if(number === 0 || isNaN(number) === true || number > 100) {
        let message = alert("The number is invalid. Enter a number between 1 to 100.");
    } else if(number <= 100) {
            resetGrids();
            makeDivs(number);
            makeGrids(number);
        }
    addHoverEffect();
    rounds++;
}

1個1個の関数がだいぶ見やすい！
最初と比べるとかなり差があるのではないか。
バグがあっても問題を追うのがだいぶ楽そうではある。

とにかく動かせるように読みやすさはあまり考えずに書いていたが、これからはより綺麗にコードを書くよう心がける。

Learning to Code

プログラミング勉強記録