by shigemk2

当面は技術的なことしか書かない

リーダブルコード 11 一度に一つのことを

前回
リーダブルコード 10 無関係の下位問題を抽出する - by shigemk2

コードは1つずつタスクを行うようにしなければならない

以下は(たとえば)ボタンをクリックすると、スコアが更新される
投票用のウィジェットのプログラムである。
ユーザは賛成(up)と反対(down)の2種類のボタンがあり、
アップで1点、ダウンで-1点とする。

var vote_changed = function (old_vote, new_vote) {
    var score = get_score();

    if(new_vote !== old_vote) {
        if(new_vote === 'Up') {
            score += (old_vote === 'Down' ? 2 : 1);
        } else if (new_vote === 'Down') {
            score -= (old_vote === 'Up' ? 2 : 1);
        } else if (new_vote === '') {
            score += (old_vote === 'Up' ? -1 : 1);
        }
    }

    set_score(score);
};

このコードだと、タイポやバグがあっても一目見ただけでは分からない。
もっと言うと、このコードは、関数内で2つのタスクを行っている。

  1. old_vote と new_vote を数値にパース
  2. scoreを更新

2つのタスクを別々に解決すれば、読みやすいコードになる。

var vote_value = function (vote) {
    if(vote === 'Up') {
        return +1;
    }
    if(vote === 'Down') {
        return -1;
    }
    return 0;
};


var vote_changed = function (old_vote, new_vote) {
    var score = get_score();
    score -= vote_value(old_vote); // 古い値を削除
    score += vote_value(new_vote); // 新しい値を削除

    set_score(score);
};

読みにくいコードがあれば、そこで行われているタスクを全て列挙する。
そこに別の関数(クラス)に分割できるタスクがあるなら関数などに分ける。