[読書録]リーダブルコード

• tmpという名前は、生存期間が短くて、一時的な保管が最も大切な変数にだけ使おう
• 悪い例(user_infoのような名前が適している)

  String tmp = user.name();
  tmp += " " + user.phone_number();
  tmp += " " + user.email();
  ...
  template.set("user_info", tmp);

• 良い例（tmpの生存期間が数行）

  if (right < left) {
  	tmp = right;
  	right = left;
  	left = tmp;
  }

• 悪い例(ミリ秒に関する挙動が不明瞭)

  var start = (new Date()).getTime(); // ページの上部
  ...
  var elapsed = (new Date()).getTime() - start; // ページの下部
  document.writeln("読み込み時間" + elapsed + " 秒");

• 良い例(ミリ秒に関する挙動が明瞭)

  var start_ms = (new Date()).getTime(); // ページの上部
  ...
  var elapsed_ms = (new Date()).getTime() - start_ms; // ページの下部
  document.writeln("読み込み時間" + elapsed_ms / 1000 + " 秒");

• 「選択する」のであれば、select()にしたほうがよい。「除外する」のであれば、exclude()にしたほうがよい
• 限界値を含めるときはminとmaxを使う
• 範囲を指定するときはfirstとlastを使う
• 包含/排他的範囲にはbeginとendを使う
• ブール値の変数名は、頭にis,has,can,shouldなどをつけてわかりやすくすることが多い

コメントの目的は、書き手の意図を読み手に知らせることである

コードからすぐにわかることをコメントに書かない

• 悪い例

  # 2番めの'*'以降をすべて削除する
  name = '*'.join(line.split('*')[:2])

ひどい名前はコメントをつけずに名前を変える

• 悪い例

  // Replyに対してRequestで記述した制限を課す
  // 例えば、帰ってくる項目数や合計バイト数など
  void CleanReply(Request request, Reply reply);

• 良い例

  // 'replay'を'request'にある項目数やバイト数の制限に合わせる
  void EnforceLimitsFromRequest(Request request, Reply reply);

「監督のコメンタリー」を入れる

コードの欠陥にコメントをつける

定数にコメントをつける

• 良い例

  // メールを送信する外部サービスを呼び出している（1分でタイムアウト)
  void SendEmail(string to,string subject,string body);

• 悪い例

//データをキャッシュに入れる。ただし、先にそのサイズをチェックする

• 良い例

//データをキャッシュに入れる。ただし、先にデータのサイズをチェックする
もしくは、「それ」を明確にして、
// データが十分に小さければ、それをキャッシュに入れる

• 関数の動作はできるだけ正確に説明する。
• コメントに含める入出力の実例を慎重に選ぶ。
• コードの意図は、詳細レベルではなく、高レベルで記述する。
• よくわからない引数にはインラインコメントを使う（例：Function(/_ arg = _/ … )）。
• 多くの意味が詰め込まれた言葉や表現を使って、コメントを簡潔に保つ。
  • 例)「正規化」「ヒューリスティック」「ブルートフォース」「ナイーブソリューション」

条件やループなどの制御フローはできるだけ「自然」にする

• 良い例

  if(length >= 10)

• 悪い例

  if(10 <= length)

• 指針

  左側	右側
  「調査対象」の式。変化する	「比較対象」の式。あまり変化しない

条件は否定形よりも肯定形を使う。例えば、 if (!debug) ではなく、 if (debug) を使う。

単純な条件を先に書く

関心を引く条件や目立つ条件を先に書く

基本的にはif/elseを使おう。三項演算子はそれによって簡潔になるときにだけ使おう

コードは上から下に読んでいくので、do/whileは少し不自然だ。コードを2回読むことなってしまう。

• whileループにすれば、コードブロックを読む前に繰り返しの条件がわかるので、読みやすくなる

ネストの深いコードは理解しにくい

ネストを削除するには「失敗ケース」をできるだけ早めに関数から返せばいい

悪い例

for (int i = 0; i < result.size(); i++) {
	if (result[i] != NULL) {
		non_null_count++;

		if(result[i]->name != "") {
			cout << "Considering candidate..." << endl;
			...
		}
	}
}

良い例

for (int i = 0; i < result.size(); i++) {
	if (result[i] == NULL) continue;
	non_null_count++;

	if(result[i]->name == "") continue;
	cout << "Considering candidate..." << endl;

	...
}

式を表す変数を使えば良い。この変数を説明変数と呼ぶこともある

• 説明変数の例

  if line.split(':')[0].strip() == "root":	
  ...

  を説明変数を使えば、

  username = line.split(':')[0].strip()
  if username == "root":
  ...

  となる。

大きなコードの塊を小さな名前に置き換えて、管理や把握を簡単にする変数のことを要約変数と呼ぶ

• 要約変数の例

  if (request.user.id == document.owner_id){
  	// ユーザーはこの文書を編集できる
  }
  ...
  if (request.user.id != document.owner_id){
  	// 文書は読み取り専用
  }

  のコードは、変数が5つも入っているから、考えるのにちょっと時間がかかる。このコードが言いたいのは、「ユーザーは文書を所持しているか？」なので、要約変数を追加すると、すると、この概念をもっと明確に表現できる。

  final boolean user_owns_document = (request.user.id == document.owner_id);
  
  if (user_owns_document){
  	// ユーザーはこの文書を編集できる
  }
  ...
  if (!user_owns_document){
  	// 文書は読み取り専用
  }

ドモルガンの法則を使う

• 例

not (a or b or c) ⇔ (not a) and (not b) and (not c)
not (a and b and c) ⇔ (not a) or (not b) or (not c)

「頭がいい」コードに気をつける。あとで他の人がコードをよむときにわかりにくくなる

役に立たない一時変数

• 悪い例

  now = datetime.datetime.now()
  root_messsage.last_view_time = now

  • 複雑な式を分割していない
  • より明確になっていない。datetime.datetime.now()のままでも十分に明確だ
  • 一度しか使っていないので、重複コードの削除になっていない

制御フロー変数を削除する

• 悪い例

  boolean done = false;
  while (/* 条件 */ && !done) {
  	...
  	if(...) {
  		done = true;
  		continue;
  	}
  }

• 良い例

  while (/* 条件 */) {
  	...
  	if(...) {
  		break;
  	}
  }

• うまくプログラミングすれば、制御フロー変数は削除できる

変数のことが見えるコード行数をできるだけ減らす

• グローバル変数に限らず、すべての変数の「スコープを縮める」のはいい考えだ
• 悪い例

submitted = false; // 注意：グローバル変数

var submit_form = function (forrm_name) {
	if (submitted) {
		return false; // 二重投稿禁止
	}
	...
	submitted = true;
}

}

• 良い例

var submit_form = (function () {
	var submitted = false; // 注意：以下の関数からしかアクセスされない

	return function (form_name) {
		if (submitted) {
			return false; // 二重投稿禁止
		}
		...
		submitted = true;
	};
}());

JavaScriptでは、変数の定義にvarをつけないと、その変数はグローバルスコープに入ってしまう

• 例

  <script>
  	var f = function () {
  		// 危険: 'i'は'var'で宣言されていない！
  		for (i = 0; i < 10; i += 1) ...
  	};
  	f();
  </script>
  
  <script>
  	alert(i); // '10'が表示される。'i'はグローバル変数なのだ
  </script>

• JavaScriptの「ベストプラクティス」は、「変数を定義するときには常にvarキーワードをつける」

C++やJavaのような言語にはブロックスコープがある。if/for/tryなどのブロックで定義された変数は、スコープがそのブロックに制限される

変数を操作する場所が増えると、現在地の判断が難しくなる

• 変数は一度だけ書き込む

与えられた地点から最も近い場所を見つける

• 悪い例

  // 与えられた緯度経路に最も近い’array’の要素を返す
  // 地球が完全な球体であることを前提としている
  var findClosestLocation = function (lat, lng, locations) {
  	var closest;
  	var closest_dist = Number.MAX_VALUE;
  	for (var i = 0; i < array.length; i += 1) {
  		var lat_rad = radians(lat);
  		var lng_rad = radians(lng);
  		var lat2_rad = radians(array[i].latitude);
  		var lng2_rad = radians(array[i].longitude);
  		
  		// 「球面三角法の第二余弦定理」の公式を使う
  		var dist = Math.acos(Math.sin(lat_rad) * Math.sin(lat2_rad) +
  												Math.cos(lat_rad) * Math.cos(lat2_rad) *
  												Math.cos(lng2_rad - lng_rad));
  		if (dist < closest_dist) {
  			closest = array[i];
  			closest_dist = dist;
  		}
  	}
  	return closest;
  };

• 良い例

  var spherical_distance = function (lat1, lng1, lat2, lng2) {
  	var lat1_rad = radians(lat1);
  	var lng1_rad = radians(lng1);
  	var lat2_rad = radians(lat2);
  	var lng2_rad = radians(lng2);
  
  	// 「球面三角法の第二余弦定理」の公式を使う
  	return Math.acos(Math.sin(lat1_rad) * Math.sin(lat2_rad) +
  										Math.cos(lat1_rad) * Math.cos(lat2_rad) *
  										Math.cos(lng2_rad - lng1_rad));
  };
  
  var findClosestLocation = function (lat, lng, array) {
  	var closest;
  	var closest_dist = Number.MAX_VALUE;
  	for (var i = 0; i < array.length; i += 1) {
  		var dist = spherical_distance(lat, lng, array[i].latitude, array[i].longitude);
  		if (dist < closest_dist) {
  			closest = array[i];
  			closest_dist = dist;
  		}
  	}
  	return closest;
  };

• 関数を抽出することで、難しそうな幾何学の計算に心を奪われることなく、高レベルの目標に集中できる
• さらに言うと、spherical_distance()は個別にテストができる関数だ

プロジェクト固有のコードから汎用コードを分離する

• 一般的な問題を解決するライブラリやヘルパー関数を作っていけば作っていけば、プログラムに固有の小さな核だけが残る

小さな関数を作りすぎると、逆に読みにくくなってしまう

ブログに設置する投票用のウィジットの例(vote_changed)

• たった1つのこと（スコアを更新すること)をしているように見えて、実際には一度に2つのタスクを行っているので、別々のタスクを解決させる
• 悪い例

  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);
  };

• 良い例

  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);
  };

読みにくいコードがあれば、そこで行われているタスクをすべて列挙する。そこには別の関数（やクラス）に分割できるタスクがあるだろう

1. コードの動作を簡単な言葉で同僚にもわかるように説明する
2. その説明のなかで使っているキーワードやフレーズに注目する
3. その説明に合わせてコードを書く

• プログラムのデバッグに悩む学生は、部屋の隅に置かれたテディベアに向かって最初に説明しなければいけない
• この技法は「ラバーダッキング」とも呼ばれている

1. 汎用的な「ユーティリティ」コードを作って、重複コードを削除する 1.未使用のコードや無用の機能を削除する
2. プロジェクトをサブプロジェクトに分割する
3. コードの「重量」を意識する。軽量で機敏にしておく

1. 不必要な機能をプロダクトから削除する。過剰な機能は持たせない
2. 最も簡単にあ問題を解決できるような要求を考える
3. 定期的にすべてのAPIを読んで、標準ライブラリに慣れ親しんでおく

他のプログラマが安心してテストの追加や変更ができるように、テストコードを読みやすくする

一般的な設計原則として、「大切ではない詳細はユーザから隠し、大切な詳細は目立つようにする」 べき

8つの問題があるテストコード

void Test1() {
	vector<ScoredDocument> docs;
	docs.resize(5);
	docs[0].url = "http://example.com/";
	docs[0].score = -5.0;
	docs[1].url = "http://example.com/";
	docs[1].score = 1;
	docs[2].url = "http://example.com/";
	docs[2].score = 4;
	docs[3].url = "http://example.com/";
	docs[3].score = -99998.7;
	docs[4].url = "http://example.com/";
	docs[4].score = 3.0;

	SortAndFilterDocs(&docs);

	assert(docs.size() == 3);
	assert(docs[0].score == 4);
	assert(docs[1].score == 3.0);
	assert(docs[2].score == 1);
}

• 問題点
  1. このテストには、どうでもいいことがたくさん書かれている。テストステートメントはあまり長くしてはいけない
  2. テストが簡単に追加できない
  3. 失敗メッセージが役に立たない。デバッグに使える情報が足りない
  4. 一度にすべてのことをテストしようとしている。テストは分割したほうが読みやすい
  5. テストの入力値が単純ではない。-99998.7は「大音量」で目立つけど、これといって意味はない
  6. テストの入力値が不完全である。例えば、スコアが0の文書をテストしていない
  7. 極端な入力値を使ってテストしていない。例えば、空のベクタ・巨大なベクタ・スコアが重複したもの
  8. Test1()という意味のない名前がついている。テスト関数の名前は、テストする環境や状況を表したものにすべき

テストに集中しすぎてしまう可能性 - テストのために本物のコードの読みやすさを犠牲にしてしまう - テストのカバレッジを100%にしないと気がすまない - テストがプロダクト開発の邪魔になる

class MinuteHourCounter {
	public:
		// カウントを追加する
		void Count(int num_bytes)
		// 直近1分間のカウントを返す
		int MinuteCount();
		// 直近1時間のカウントを返す
		int HourCount();
};

• 問題点
  • 名前を改善する
  • コメントを改善する

// 直近1分間および直近1時間の累積カウントを記録する。
// 例えば、帯域幅の仕様状況を確認するのに使える。
class MinuteHourCounter {
	// 新しいデータ点を追加する（count >= 0)。
	// それから1分間は、MinuteCount()の返す値が+countだけ増える。
	// それから1時間は、HourCount()の返す値が+countだけ増える。
	void Add(int count);

	// 直近60秒間の累積カウントを返す
	int MinuteCount();

	// 直近3600秒間の累積カウントを返す
	int HourCount();
};

• Code Complete
• リファクタリング
• プログラミング作法
• 達人プログラマー
• Clean Code

• JavaScript: The Good Parts
• Effective Java
• オブジェクト指向における再利用のためのデザインパターン
• 珠玉のプログラミング
• ハイパフォーマンスWebサイト
• Joel on Software

• ライティングソリッドコード
• ケント・ベックのSmalltalkベストプラクティス・パターン
• プログラム書法
• 文芸的プログラミング

1. 実際にやる
2. 当たり前にする
3. コードで伝える