【コードレビュー】始めよう読みやすいコード作り

はじめに

コードレビューをする機会があり、まずは「読みやすいコード」に注目しました。
なぜなら、実務の現場でおいて書いたコードは本人以外が見ることが多くあります。
また、自分自身が読み直すことも多いでしょう。
その際にコード解読に時間をかけるのはナンセンスです。

しかし、「読みやすいコード」を感覚的に指摘するわけにもいきません。
そこで私自身がうまく言語化するために「リーダブルコード」を読み直しました。

本書は四部構成となっています。

1. 表面上の改善
2. ループとロジックの単純化
3. コードの再構成
4. 抜粋テーマ

今回は「第一部：表面上の改善」で紹介されているものから抜粋しました。
今日からできる表面的な事項についてまとめましたので参考にしてください。
https://www.oreilly.co.jp/books/9784873115658/

表面上の改善

「表面上」の改善は読みやすいコードの第一歩です。

• 適切な命名
• 優れたコメント文
• コードをキレイにフォーマット

以上の三点はロジック部分の変更やリファクタリングをしなくても改善ができます。
少しづつメソッド単位やクラス単位で「読みやすいコード」に変えることができます。

「第一部：表面上の改善」から以下を抜粋します

• 第二章：名前に情報を詰め込む
• 第三章：誤解されない名前
• 第五章：コメントすべきことを知る
• 第六章：コメントは正確で簡潔に

1. 名前に情報を詰め込む

1.1 明確な単語を選ぶ

この関数名からは何かしらページを取得することが推測できます。

しかし、GetPageという命名だと「どこから」が抜けているため、以下のような疑問が出てくる場合があります。

• ローカルキャッシュから？
• データベースから？
• インターネットから？

仮にインターネットからページを取得するのであれば、FetchPageやDownloadPageといった命名がより明確です。

もう一例としてStop()という関数を挙げます。
この場合ですと動作に合わせてより明確な命名をつけることができます。

• 取り消しのきかない重い操作：Kill()
• あとから再開できる操作：Pause()

といったように状況に応じて使える単語を選択しましょう。
参考資料として「シソーラス（類語辞典）」を活用しましょう。

単語	代替案
send	delivert, dispatch, announce, route
find	serch, extract, locate, recover
start	launch, create, begin, open
make	create, set, up, build, generate, new

近年ですとChatGPTを活用する方法もあります。

私自身も何かしらを取得する関数やメソッドの場合、Getを用いることが多いです。
今一度、命名時には立ち止まって考えていきたいです。

1.2 tmp、retvalなどの汎用的な名前を避ける

tmpやretval、fooなどの命名では無く、エンティティの値や目的を表した名前を選ぼう。
戻り値にretvalを用いるのでは無く、何を戻すのかを表そう。（戻り値がretvalなのは当たり前である）

// [Bad]
retval += v[i]

// [Good]
sum_squares += v[i] // 二乗を合計するが、二乗されていない！！不具合だ！

retvalとい名前には情報が存在しない。変数の値を表すような名前を使おう。
[Good]のケースでは、その結果から不具合であることが気づけるようになる。

汎用的な名前を使用してはダメということではない。
tmpを使った上手な使用例：

//[Good]
// 2つの変数の中身を入れ替えるコード
if(right < left){
    tmp = right
    right = left
    left = tmp
}

tmpの生存期間が短く、文字通り一時的な保管として意味合いのある場合である。

//[Bad]
String tmp = user.name
tmp += " " + user.email
tmp += " " + user.age
template.set("user_info", tmp)

生存期間は短いが、tmpが何度も書き換えられており「一時的な保管」ではない。
この場合はuser_infoといった変数名が適当である。

1.3 名前に情報を追加する

変数名は短いコメントのようなものです。
もし知らせないといけない大切な情報があれば「単語」を変数名に追加しましょう。
例に16進数で管理する変数を挙げます。

var id int // 0xf0a2

「id」というフォーマットが大切な場合、hex_idにすると良いでしょう。
[値の単位]
時間やバイト数のように計測可能なものは変数名に単位を入れるといいでしょう。
単位を追加することで、「どのくらい」が容易に分かるようになります。

	単位を追加した例
遅延時間 delay	delay_sec, delay_msec
ファイルサイズ size	size_gb, size_mb, size_kb, size_byte

[重要な属性]
変数名に追加できる情報は単位だけではありません。
危険や注意を示す状況や状態を追加することもできる。

状況	変数名	改善
暗号化前のパスワード	password	plaintext_password
エスケープ前のユーザ入力コメント	comment	unescaped_comment
HTMLの文字コードをUTF-8へ変更した	html	html_utf8

これらは必ず追加する必要は無い。
変数の意味を間違えてしまったときにバグや不具合として露見しそうなところに使用するとよい。
基本的には、変数名の意味を理解していないと困る場合に属性を追加するとよい。

2. 誤解されない名前

前章では、名前に情報を詰め込むことに触れました。
その際には「誤解されない情報」を詰め込まなければなりません。
本章は、そんな誤解されない名前について述べられている。

誤解されない名前にするには、積極的に「誤解」を探していく必要があります。
そうすることで変更が必要なあいまいな命名を見つけることができるだろう。

名前が「他の意味と間違えられることはないだろうか？」と何度も自問自答する。

以降に例をあげていく。

2.1 限界値を含めるときはminとmaxを使う

// BAD
const ITEMS_IN_CART_LIMIT = 42

// GOOD
const ITEMS_IN_CART_MAX = 42

limitという言葉はあいまいです。限界という意味合いがありますが、「未満」なのか「以下」なのかが明確にわかりません。
限界値はmaxやminで表現することでより明確な命名となります。

2.2 範囲を指定するときはfirstとlastを使う

「未満」と「以下」の例は範囲の指定にもありうる。
[Bad]

array := []int{0, 1, 2, 3, 4, 5, 6}
start := 2
stop := 3
// 範囲の値を出力
printRange(array, start, stop)

startは適当な命名であります。しかし、stopはどうでしょうか？
指定範囲が[2, 3]なのか[2, 3, 4]なのかが明確には分からない。

[Good]

array := []int{1, 2, 3, 4, 5, 6}
first := 2
last := 3
// 範囲の値を出力
printRange(array, first, last)

終端を範囲に含めるのであれば、lastがよりよい。
意味的に正しく解釈するのであれば、maxやminを使った場合もよいでしょう。

2.3 ブール値の名前

bool型（true、false）の変数や値を返す関数の名前を選ぶ場合、trueとfalseの意味を明確にする必要があります。

var readPassword bool = true

ここではreadという命名は避けるのが好ましい。

• パスワードをこれから読み取る必要がある
• パスワードをすでに読み取っている

という解釈が分かれる場合がある。

代わりにneedPasswordやuserIsAuthenticatedを用いるのが良い。
bool型の変数名は接頭辞にis、has、can、shouldなどをつけて明確にすることが多い。
また命名は肯定形で定義することが望ましい。

3. コメントすべきことを知る

コメントを多く書くと一見便利そうですが画面を占有したり、コメントを読む分だけコードを読む時間が減ります。
そのため、コメントにはその分の価値を持たせる必要があります。
コメントは書き手の意図を読み手に知らせるために記述します。

最初にコメントすべきでは「ない」ことを２点、後にコメントすべきことについて２点述べる。

3.1 コメントすべきでは「ない」こと

以下のコメントはコメントすべきで「ない」ことになる。
コードからすぐわかることはコメントをしない

// ユーザ名を取得する 
name = GetUserName()

// ユーザ名が空の場合、ユーザ名をセットする
if name == "" {
    name = SetUserName()
}

コメントの付与によって新しく情報が提供されているわけでもなく、読み手がりかいしやすくなるわけでもない。
そのため、「価値の無い」コメントといえる。

コメントのためのコメントをしない

// 与えられた文字列sを反転する関数
func reverseString(s []string) {
    ................
}

これも「価値の無い」コメントといえる。
コメントをつけるのであれば、もっと大切なことを説明したほうが良い。

// 【制約】1 <= s.length <= 200
func reverseString(s []string) {
    ................
}

関数名は「自己文書化」されている必要がある。
コメントで名前の埋め合わせをせず、関数名を変えよう。

3.2 コメントすべきこと

以下からはコメントすべきことを述べる。
自分の考えを記録する

// 改造が重なりと当初から汚くなっている。
// HACK：テストが容易になるよう、関数をAとBの２つに分けたほうが良い。
func XXXXXX() {
    ................
}

このコメントでコードが汚いことを認めている。そして修正案を述べ、修正を促している。
このコメントが無ければ誰も近づくことなく、苦労してテストをおこなうことになるだろう。

またこのように改造が必要な場合は、以下のようなコメントを残すとあとから残課題が発見しやすい。

記法	典型的な意味合い
TODO:	あとで手をつける
FIXME:	既知の不具合があるコード
HACK:	あまり綺麗じゃない解決策
XXX:	危険!大きな問題がある

うまく書けないときは自分の考えていることを書き出してみよう

定数にコメントをつける
定数を定義するときは、その定数が何をするのか、なぜその値を持っているのかという「背景」が存在することが多い。

NUN_THREADS 8

このコードには特にコメントは必要ないようにみえる。しかし、これを書いたプログラマは何か考えがあるかもしれない。

NUN_THREADS 8 // 値は「 >= 2 * num_processors」で十分

これなら値の決め方がわかる（1だと小さすぎて、50だと大きすぎる）。
また値が厳密じゃなくてもいい場合もある。この場合もコメントが大いに役立つ。

定数を決めた時に考えていたことをコメントに残してみよう

読み手の立場になって考える
[全体像についてのコメント]
新しいチームメンバにとって、最も難しいのは「全体像」の理解である。
クラスの連携やデータの流れ、エントリポイントなど、こうしたコメントは残されていないことも多い。
メンバアサイン初期に口頭で以下のようなことを伝えていないだろうか？

• これはロジックとDBをつなぐクラスです。直接は利用しないでください。
• これはいろいろ難しいことをやっていますがただのキャッシュです。システムにはあまり関与していません。

簡単な会話ですが、ソースコードを読んだだけでは得られない情報がありました。
これはコメントに書くべき情報です。
制約条件を難しく正確に書く必要はありません。数行あるだけでなにもないよりはマシです。

[要約コメント]
関数内部でも「全体像」についてコメントをするのはいい考えです。

    // 顧客が自分で購入した商品を検索する
	for _, customerID := range allCustomers {
		for _, sale := range allSales[customerID] {
			if sale.Recipient == customerID {
            .................
            }
        }
    }

コメントが無かった場合、なんのためにallCustomersを走査するのか分からなくなる。

普段とは違う場合は特にその理由書くと良い
例：慣習的に条件比較を==が使うが特別に!=にしている場合のWHY。

また関数内部の処理を区分をつけ、塊単位にコメントを付与してもよい。

func GenerateUserReport() {
    // ユーザロックを取得する
    .................

    // ユーザ情報をDBから読み込む
    .................

    // 情報をファイルに書き出す
    .................

    // ユーザロックを解放する
    .................

}

4. コメントは正確で簡潔に

コメントは領域に対する情報の比率が高くなければいけない。

4.1 あいまいな代名詞を避ける

コメント中の「それ」や「これ」といった指示語は何を指し示すのかよくわからないことがあります。

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

「その」が指していいるのは、「データ」かもしれないし「キャッシュ」かもしれない。
コードを読み進めれば、どちらを指しているかはわかるだろう。
しかし、そうしないと分からないのであればなんのためのコメントを書いているのだろうか。
こういう場合は、名詞を代名詞に代入するとよい。
今回のケースでいうと「その」を「データ」に変えてみるのだ。

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

4.2 歯切れの悪い文章を書く

コメントを正確にすることと簡潔にすることは両立することが多い。
以下はウェブクローラを例に挙げる。

// これまでにクロールしたURLかどうかによって優先度を変える。

一見問題なさそうに見える。しかし、以下と比較するとどうでしょうか？

// これまでにクロールしていないURLの優先度を高くする。

こちらのほうが単純で短く直接的と感じるだろう。
さらに「優先度」をどうするのかといった情報も含まれている。

4.3 関数の動作を正確に記述する

ファイルの行数を数える関数を書いているとします。

// このファイルに含まれている行数を返す。
func CountLines(fileName string) int {
    .....................
}

あまり正確なコメントではない。「行」には様々な意味があります。
コーナーケースを考えると見えてくるものがあります。

• ""（空ファイル）は、0行なのか、1行なのか
• "hello"は、0行なのか、1行なのか
• "hello\n"は、1行なのか、2行なのか
• "hello\n world"は、1行なのか、2行なのか
• "hello\n world\n\r hogehoge\r"は、2行なのか、3行なのか、はたまた4行なのか

仮に"\n"の数を数えるように実装します。
その際のコメントが下記になる。

// このファイルに含まれている改行文字"\n"を数える。
func CountLines(fileName string) int {
    .....................
}

最初のコメントより文字数はさほど変わらないが、情報量は格段に増えている。
コーナーケース時の結果が想像できるようになった。

さいごに

リーダブルコードを読み直しましたが、やはり一度にすべて意識づけるのは難しいと感じました。
特に後半になるにつれ、難易度が上がります。
そのため、今回紹介した「表面上の改善」の内容が実施出来たら「ループとロジックの単純化」、「コードの再構成」...と続項目の意識付けをやっていこうと思います。

https://www.oreilly.co.jp/books/9784873115658/