前回
リーダブルコード 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つのタスクを行っている。
- old_vote と new_vote を数値にパース
- 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); };
読みにくいコードがあれば、そこで行われているタスクを全て列挙する。
そこに別の関数(クラス)に分割できるタスクがあるなら関数などに分ける。