リーダブルコードまとめ
趣旨
読んだ技術書の内容を、備忘録としてまとめておきます。
完全な要約ではなく、自分が感心した内容を中心に記述していきます
🛠️ 表面上の改善
🏷️ 名前に情報を詰め込む
明確な単語を選ぶ
- 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
のいずれかが存在しない可能性がある。
- 「都市」に
LocalityName
→SubAdministrativeAreaName
→AdministrativeAreaName
の順で使用可能なものを使う - 全て使えない場合、「都市」に
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, USA
→Santa Monica, California
次のようにタスクを分けてみる
- location_infoディクショナリから値を抽出する
- 「都市」の優先順位を調べる。見つからない場合、デフォルトの
Middle-of-Nowhere
- 「国」を取得する。なければ
Planet Earth
- 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;
その他の手法
- 国がUSAならば別の変数を使う
- 先ほどのコードには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