【プログラミング】割とやりがちな見づらいコード3選【初級者向け】 │ コジマノテック

1. はじめに
2. １：ネストが深い
3. ２：マジックナンバー大集結
4. ３：関数化されず冗長
5. リファクタしてみよう
6. さいごに

はじめに

コジマです。

初心者じゃなくても結構いるんですよね。

特に保守として既存コード引き継いだときとか、なんでこんなコード書いてるんや…!!!!!!!!!

ってなることが結構あるので、特に気になる３つの特徴について話したいと思います。

これはscoreに応じてセリフを出しわけるソースです。
見るだけで気持ち悪くなりそうですね。

const score = 80; // 点数

if(score > 50){
    if(score > 70){
        if(score > 90){
            console.log('Very Good!')
        }else{
            console.log('Good!')
        }   
    }else{
        console.log('Not Bad.')
    }
}else{
    console.log('OMG!')
}

何が悪いのか掘り下げていきましょう。

１：ネストが深い

ifつなげてればどうにかなると思ってるor書いてるうちは読めている
そんな時に起こりやすいです。
また、for文と組み合わさると頻度が爆上がりします。

深いネストはとにかく読みづらいです！
ネストは少なくしましょう！マジで！

関数化してさっさとリターンするか、switch-caseするとよいです。

この例の場合はさっさとリターンしちゃいたいですね。（関数化については後述）

switch-caseは好みにもよると思いますが、私の場合は
1のときはこう、2のときはこうって感じで離散的な値を場合分けするときによく使います。

２：マジックナンバー大集結

意味のある数値がハードコーディングしてある状態のものをマジックナンバーと言います。

この例だと、50,70,90というのがハードコーディングされています。
どこかで仕様が変わって、50が60に変わるかもしれません。
数値は変わりますが、その数値が持つ意味は変わりません。
こういった場合は定数化してしまったほうが見通しがよくなります。

50は悪くない点数（NOT_BAD_SCORE）
70はよい点数（GOOD_SCORE）
90はとても良い点数（VERY_GOOD_SCORE）
としてみましょう。

３：関数化されず冗長

もしシステムが拡大され、例の処理を複数個所から呼び出したくなった時には
共通化するために関数化したいと思うと思います。

ただ、この処理は「点数に応じたメッセージがほしい」という気持ちで書かれた処理であって、
これは一つのまとまりとして関数化してよいはずです。

関数化することによって、上述した早期リターンをすることができるようになります。
早期リターンすることによって、無駄にソースを読む必要がなくなります。

リファクタしてみよう

上記を踏まえてリファクタしてみました。

const score = 80; // 点数

console.log(getMessage(score));

// 3:関数化
function getMessage(score){
    // 2:定数化
    const NOT_BAD_SCORE = 50;
    const GOOD_SCORE = 70;
    const VERY_GOOD_SCORE = 90;
    // 1:ネストを減らす、早期リターン
    if(score > VERY_GOOD_SCORE){
        return 'Very Good!'
    }
    if(score > GOOD_SCORE){
        return 'Good!'
    }
    if(score > NOT_BAD_SCORE){
        return 'Not Bad.'
    }
    return 'OMG!'
}

どうでしょうか？ちょっと長くはなってるけど、先ほどのよりも意味のある読みやすいソースコードの書きっぷりになったんじゃないかと思います。