📘

リーダブルコードまとめ

に公開

趣旨

読んだ技術書の内容を、備忘録としてまとめておきます。
完全な要約ではなく、自分が感心した内容を中心に記述していきます

🛠️ 表面上の改善

🏷️ 名前に情報を詰め込む

明確な単語を選ぶ

- def getPage(url):
+ def fetchPage(url):
	...

使える動詞

動詞 説明
fetch APIなどから取得する
retrieve 検索して取得する
load ローカルやストレージから読み込む
compute 計算や処理を通じて結果を取得する
generate 動的にデータを作成する

具体的な名前を使って詳細に説明する

# 任意のTCP/IPポートをサーバーがリッスンできるかを確認するメソッド
- def serverCanStart():
+ def canListenOnPort():
	...

変数名に大切な情報を追加する

値の単位

Before After
delay delay_secs(秒)
size size_mb(メガバイト)
limit max_kbps(kbps)
angle degrees_cw(度)
属性
Before After
password plaintext_password(暗号化未済)
comment unescaped_comment(エスケープ未済)
html html_utf8(文字コードをUTF8にした)
data data_urlenc(URLエンコードした)

🤔 誤解されない名前

境界値がどこか明確にする
カートに入れられるアイテム数は10が最大であることがわかりやすい

- CART_TOO_BIG_LIMIT = 10
+ MAX_ITEMS_IN_CART  = 10

範囲を指定するときはfirstとlastを使う
stopは含まれるのか曖昧

- extract(start=2, stop=4)
+ extract(first=2, last=4)

否定系よりも肯定系を使う

- bool disable_ssl = false;
+ bool use_ssl = true;

🗣️ コメントすべきことを知る

前提として、優れたコード > ひどいコード > 優れたコメントとして優先順位を考える。その上で、読み手にとって書いてあった方がいいコメントを記載する

  • 全体像を把握するコメント
def generateUserReport():
	# このユーザのロックを獲得する
	...
	# ユーザの情報をDBから読み込む
	...
	# 情報をファイルに書き出す
	...
	# このユーザのロックを解放する
	...
  • 定数の値に関する背景
# 人間の読める合理的な限界値
int MAX_RSS_SUBSCRIPTIONS = 1000;
  • 読み手がなぜ?となる箇所にコメントしておく
void Clear(){
	// ベクタのメモリを解放する(「STL swap 技法」)
	vector<float>().swap(data); // Clearなのにswap?となりそう
}

✏️ コメントは正確で簡潔に

  • 曖昧な代名詞を避ける
- データをキャッシュに入れる。ただし、先に'その'サイズをチェックする
+ データをキャッシュに入れる。ただし、先に'データ'のサイズをチェックする
  • 歯切れの悪い文章を磨く
- これまでにクロールしたURLかどうかによって優先度を変える
+ これまでにクロールしていないURLの優先度を高くする
  • 関数の動作を正確に記述する
    空のファイルが0行なのか、1行なのか
    "hello"は0行?1行?"hello\n"は?
- # このファイルに含まれる行数を返す
+ # このファイルに含まれる改行文字('\n')を数える
int count_lines(filename: str):
	...
  • 入出力のコーナーケースに実例を使う
# 実例: strip("abba/a/ba", "ab")は"/a/"を返す
def strip(src: str, chars: str):
	...

🔁 ループとロジックの単純化

🔀 制御フローを読みやすくする

比較の際、変化する値を左に、安定した値を右に配置する

- while (bytes_expected > bytes_received)
+ while (bytes_received < bytes_expected)
左側 右側
調査対象の値 比較対象の値。あまり変化しない

if/elseブロックは適切に並び替える

肯定系・単純・目立つものを先に処理する
urlにexpand_allが含まれている場合が気になるため、それを早く処理する

- if !url.has_query_param("expand_all"):
+ if url.has_query_param("expand_all"):
	...

ネストを浅くする

if user_result == SUCCESS:
	if permission_result != SUCCESS:
		reply.white_errors("error reading permissions")
		reply.done()
		return
else:
	reply.white_errors(user_result)
reply.done()

上のコードはuser_resultとpermission_resultを同時に処理しているが、2層のネストのためわかりづらい。

改善:

if user_result != SUCCESS:
	reply.white_errors(user_result)
	reply.done()
	return

if permission_result != SUCCESS:
	reply.white_errors("error reading permissions")
	reply.done()
	return

reply.white_errors("")
reply.done()

ネストレベルが1になり、だいぶ読みやすくなった

✂️ 巨大な式を分割する

巨大な式は飲み込みやすい大きさに分割する

説明変数

- if line.split(":")[0].strip() == "root":
+ user_name = line.split(":")[0].strip()
+ if user_name == "root":

要約変数

+ user_owns_document = (request.user.id == document.owner.id)
- if request.user.id == document.owner.id:
+ if user_owns_document:
	# ユーザはこの文書を編集できる
- if request.user.id != document.owner.id:
+ if !user_owns_document:
	# 文書は読み取り専用

複雑なロジックと格闘する

Rangeクラスを実装しているとし、2つのrangeに重複があるかどうかを検出するメソッドを実装しているとする。

def overlaps_width(other: Range):
# 'begin'または'end'が'other'の中にあるかを確認する
return (begin >= other.begin && begin < other.end) or
		(end > other.begin && end <= other.end) or
		(begin <= other.begin && end >= other.end)

非常に分かりづらく、バグを含んでいる可能性がある。
より優雅な方法を見つけてみる。 つまり、overlaps_widthの反対を考えると、「重ならない」になる。2つの範囲が重ならない場合は2パターンしかなく、以下の通り。

  • 一方の範囲の終点が、ある範囲の視点よりも前にある場合
  • 一方の範囲の視点が、ある範囲の終点よりも後にある場合
    実装すると以下になる
def overlaps_width(other: Range):
if other.end <= begin: # 一方の終点が、この始点よりも前にある
	return false
if other.begin >= end: # 一方の始点が、この終点よりも後にある
	return false
return true

🧮 変数と読みやすさ

変数のスコープを縮める

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

class LargeClass:
	string: str

	def method_1():
		string = ...
		method_2()

	def method_2():
		# stringを使用している

	# stringを使用していないメソッドが多くある

stringはクラス変数なのに、使用頻度が少ない。

class LargeClass:
	def method_1():
		string: str = ...
		method_2(str)

	def method_2(string: str):
		# stringを使用している

	# その他のメソッドからはstringが見えない

🧩 コードの再構成

🧹 無関係の下位問題を抽出する

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

「サーバをAjaxで呼び出してレスポンスを処理する」が高レベルの目標だが、コードの大部分は「ディクショナリをきれいに印字する」という「無関係の下位問題」を解決しようとしている。

ajax_post({
	url: "http://example.com/submit",
	data: data,
	on_success: function (response_data) {
		var str = "\n";
		for (var key in response_data) {
			str += "  " + key + " = " + response_data[key] + "\n";
		}
		alert(str + "}");
	}
});

無関係の下位問題を抽出して、format_pretty(obj)のような関数にしてみる

// 汎用的なformat_prettyを作成
var format_pretty = function (obj, indent) {
	// null,undefined,文字列,非オブジェクトを処理する
	if (obj === null) return "null";
	if (obj === undefined) return "undefined";
	if (typeof obj === "string") return '"' + obj + '"';
	if (typeof obj !== "object") return "null";

	if (indent === undefined) indent = "";
	// (非null)オブジェクトを処理する
	var str = "{\n";
	for (var key in obj) {
		str += indent + "  " + key + " = ";
		str += format_pretty(obj[key], indent + "  ") + "\n";
	}
	return str + indent + "}";
};

ajax_post({
	url: "http://example.com/submit",
	data: data,
	on_success: function (response_data) {
		alert(format_pretty(response_data));
	}
});

プロジェクトに特化した機能

business = Bussiness()
business.name = request.POST["name"]

url_path_name = business.name.lower()
url_path_name = re.sub(r"['\.]", "", url_path_name)
url_path_name = re.sub(r"[^a-z0-9]+", "-", url_path_name)
url_path_name = url_path.name.strip("-")
bussiness.url = "/biz/" + url_path_name

business.date_created = datetime.datetime.utcnow()
business.sabe_to_database()

urlはnameのクリーンバージョンなので、このコードの「無関係の下位問題」は「名前を有効なURLに変換する」になる。

business = Bussiness()
business.name = request.POST["name"]
bussiness.url = "/biz/" + make_url_friendly(business.name)
business.date_created = datetime.datetime.utcnow()
business.sabe_to_database()


CHARS_TO_REMOVE = re.compile(r"['\.]+")
CHARS_TO_DASH = re.compile(r"[^a-z0-9]+")

def make_url_friendly(text):
	text = text.lower()
	text = CHARS_TO_REMOVE.sub("", text)
	text = CHARS_TO_DASH.sub("", text)
	return text.strip("-")

🎯 一度に一つのことを

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

オブジェクトから値を抽出するタスクを考えてみる。location_infoディ区署なりに構造化された情報があり、そこからすべてのフィールドの都市、国を選んで連結する。

例:

input: {
	"LocalityName": "Santa Monica",
	"SubAdministrativeAreaName": "Los Angels",
	"AdministrativeAreaName": "California",
	"CountryName": "USA"
}
output: "SantaMonica, USA"

ややこしいのはLocalityName, SubAdministrativeAreaName, AdministrativeAreaName, CountryNameのいずれかが存在しない可能性がある。

  • 「都市」にLocalityNameSubAdministrativeAreaNameAdministrativeAreaNameの順で使用可能なものを使う
  • 全て使えない場合、「都市」にMiddle-of-Nowhereを設定する
  • 「国」にCountryNameが使えない場合、Planet Earthを設定する

以下は、値がない時の対処法の例です。

例1
input: {
	"LocalityName": undefined,
	"SubAdministrativeAreaName": undefined,
	"AdministrativeAreaName": undefined,
	"CountryName": "Canada"
}
output: "Middle-of-Nowhere, Canada"

例2
input: {
	"LocalityName": undefined,
	"SubAdministrativeAreaName": "Washington, DC",
	"AdministrativeAreaName": undefined,
	"CountryName": "USA"
}
output: "Washington, DC, USA"

このタスクを実装したコードが以下になる

var place = location_info["LocalityName"];
if (!place) {
	place = location_info["SubAdministrativeAreaName"];
}
if (!place) {
	place = location_info["AdministrativeAreaName"];
}
if (!place) {
	place = "Middle-of-Nowhere";
}
if (location_info["CountryName"]) {
	place += ", " + location_info["CountryName"];
} else {
	place += ", Planet Earth";
}
return place;

追加で要件が加わり、アメリカの場合は国名ではなく、可能であれば襲名を表示する。
例:Santa Monica, USASanta Monica, California

次のようにタスクを分けてみる

  1. location_infoディクショナリから値を抽出する
  2. 「都市」の優先順位を調べる。見つからない場合、デフォルトのMiddle-of-Nowhere
  3. 「国」を取得する。なければPlanet Earth
  4. placeを更新する
var town = location_info["LocalityName"];
var city = location_info["SubAdministrativeAreaName"];
var state = location_info["AdministrativeAreaName"];
var country = location_info["CountryName"];

var second_half = "Planet Earth";
if (country) {
	second_half = country;
}
if (state && country == "USA") {
	second_half = state;
}

var first_half = "Middle-of-Nowhere";
if (state && country !== "USA") {
	first_half = state;
}
if (city) {
	first_half = city;
}
if (town) {
	first_half = town;
}
return first_half + ", " + second_half;

その他の手法

  1. 国がUSAならば別の変数を使う
  2. 先ほどのコードにはUSAのロジックとその他が混じっていた。USAと非USAは別々に処理できる
var town = location_info["LocalityName"];
var city = location_info["SubAdministrativeAreaName"];
var state = location_info["AdministrativeAreaName"];
var country = location_info["CountryName"];

var first_half, second_half;

if (country === "USA") {
	first_half = town || city || "Middle-of-Nowhere";
	second_half = state || "USA";
} else {
	first_half = town || city || state || "Middle-of-Nowhere";
	second_half = country || "Planet Earth";
}
return first_half + ", " + second_half;

📏 短いコードを書く

最も読みやすいコードは、何も書かれていないコードだ
プロジェクトを始めるときには、ファイルは数える程度で、関数やクラスを把握しやすい。
しかしプロジェクトが進んでいくとディレクトリを分け、ファイルを整理する必要が出てくる。全体を把握することは難しくなってくる。
つまり、プロジェクトが成長しても、コードをできるだけ小さく軽量に維持するべき。

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

Discussion