コードレビュー中.....
リーダー：「なあわい君。このコードの意味ってどういう事なんや？」
わい：「ああ。これはなえーとなんやったけ？○×△☆♯♭●□▲★※的な事をやってんねん」
リーダー：「いや自分でもよく分からなくなってるやんw」
リーダー：「基本的にコードを書くっていうことは、見やすいことだけじゃなく、相手に分かりやすいかどうかを重視するんや。あくまで実行するのはコンピューター側だけど、読むのは人間なんやで？」
リーダー：「てことで、わい君にこの本を授けるわ。休日読んでや」
リーダー：「テレレレッテレ～♪『リーダブルコード より良いコードを書くためのシンプルで実践的なテクニック』」（大山のぶ代ボイス）」
わい：「なんやこいつ（あ、ありがとうやで〜）」
リーダブルコード
この記事では、わいが読んだ『リーダブルコード より良いコードを書くためのシンプルで実践的なテクニック』を読んだ後のアウトプットがてらの記事です。
随時更新していきます。
※記載は2020-12-22時点で3章まで
【リーダブルコード】第1章 理解しやすいコード
ここはほとんど前置き。
Q. 「理解しやすいコード」ってなんぞや
A. 相手が最短で理解できるコード が「理解しやすいコード」
自分だけが理解できてもダメ。
相手が読んで確実に理解ができるコードを書くことが絶対。
「理解しにくいコード」は相手に確認の時間を取らせたり、調査の時間に取られてたり、結果的に相手の時間を想像以上に奪ってしまうことに繋がる。それだけでも避けなくてはならない。さらに、理解しやすいコードは、良い設計、良いテストに繋がる。まずここを重視しなければならない。
って感じの前置きが書かれています。
よく"クソース"なんて呼ばれるものは定義が色々あると思いますが、リーダブルコードでは「1, 2」のことを言っている気がする。
- 処理が効率的ではない（実は便利な関数があったりとか、知見が無くて無理矢理自力で書いた的なやつ
- 書いてる処理がよく分からない（何がしたいんやってやつ
- 書き方がモダンじゃない（プログラミング言語のver次第で出来ることもある的なやつ
【リーダブルコード】第2章 名前に情報を詰め込む
よくプログラミングをしている時に使用する用語が多々あります。
その時は以下のQiitaの記事を見ると参考になることが多いです。
プログラミングでよく使う英単語のまとめ【随時更新】
ここで言う名前と呼ばれる物はコードに出てくる全ての物。
- クラス名
- 変数名
- メソッド名
ここの名前に情報を埋め込むと言う事。
クラス名・変数名・メソッド名の選び方は以下び6つを気を付ければOK
- 明確な単語を選ぶ
- 汎用的な名前を避けるか、使うのであれば状況を選ぶ
- 抽象的な名前ではなく、具体的な名前を使う
- 接尾辞や接頭辞を使って情報を追加する
- 名前の長さを決める
- 名前のフォーマットで情報を伝える
1. 明確な単語を選ぼう
よくある「get」という単語。コード上だと思いの外便利な単語ですよね。
カプセル化のGetter, Setterの概念からgetHogeHogeなどメソッド名など使う場面など多いと思います。
単純にデータ取得の部分や、コンバート処理の結果を取得したい時なども汎用的に付けれてしまいますが、それは適切ではないらしい。
例えばいきなりこのコードを見て、初見の人はどう思うのか。
public function getPage($url)
このページってどこから取得するのか皆目検討も付かないですね。
クラス名から推測も出来ますが、初見でいきなり見ても、キャッシュデータから？データベースから？外部から？
外部にアクセスして取得するのであれば**fetchPageやDownloadPage**などの方が適切。
また、クラス名からも推測が難しい場合があります。
class Binary
{
public function getSize()
{
}
}
何のSize？ってことになる。
メモリに関することなら**getMemoryBytes、ノードの数などであればgetNumNodes**が適切。
2. 汎用的な名前を避けよう
- tmp
- retval
- foo
こういった変数名を見た事多いのでは。
例えば、うちの会社の最古のコードの部分にはtmpといった名前がちらほらあります。
主に待避用の変数名として用意されてる節がある。
ただ、これは如何にも退避と分かり、別段初見でわかれば問題はないそう。例えば以下のようなやつ。
if ($right < $left) {
$tmp = $right;
$right = $left;
$left = $right;
}
ただ、こう言った待避とは別のコードに使うのはやめた方が良い。
「待避用のデータで保存している...？」と誤解を生んでしまう。
$tmp = Templpate::tempolaryFile();
*** 省略
$this->saveData($tmp)
あくまで**tmpといった変数名は生存期間が短く、一時的な保存の役割**だけに使った方が良さげです。
また、「i, j,k」などのループイテレータの変数も気をつけようと書いてあります。
ループなどの入れ子状態になった際に、イテレーターがどこの値なのか分かりやすくするためです。
// こういうのではなく
for (let i = 0, member.lenght(), i++)
// こっちの方が良い
for (let member_i = 0, member.lenght(), member_i++)
for (let mi = 0, member.lenght(), mi++)
3. 抽象的な名前を避けよう
例えば、ServerCanStart()というメソッドがあったとする。
コードの書き手としては、TCP/IPポートをサーバーがリッスンできるかどうかの確認などに使用するメソッドらしい。しかし、サーバーがリッスンできるかどうかというのが具体的な行動なので、そこの情報を名前に入れ込んでCanListenOnPort()とかの方が一発で分かる。
上記のように具体的な行動を示すものがあるならば、そこの部分から情報を開示してあげるのが一番良いみたい。（確かに）
ただ、名前に情報を埋め込むには限界がある。そんな時は「単語」をうまく使うと良い。
例えば、以下のようなメソッドがある場合。
let now = new Datetime();
let time = now.getTime();
JavaScriptで実際にはnowという変数にはDatetimeオブジェクトが、timeには現在時刻をミリ秒(ms)で情報を返してくれる。このDatetimeオブジェクトの仕様に慣れていればなんて事ない処理だけど、初見でこのtimeに何の値が入ってくるのか。
これだけでは皆目検討も付かない。なので、現在時刻をミリ秒(ms)で情報を返してくれるという、この**「ミリ秒」という単語**に注目して変数名を変えてあげると良い。
let now = new Datetime();
let timeMs = now.getTime();
他にも以下のような事例がある。
単位を付与することにより、具体的にイメージする事ができる。
-
Start(int delay)→Start(int delay_sec)秒数 -
CreateCache(int size)→CreateCache(int size_mb)メガバイト -
PageDownload(int limit)→PageDownload(int limit_kbps)キロビーピーエス
また、重要な属性を追加することにも気をつけた方がいい。
例えば、passwordという変数名があったとする。
- すでに暗号化されている場合：
encryptedPassword - まだ暗号化されていない平文の場合：
plaintextPassword
といった属性値を情報として追加すると、より一層良いものになる。
このplaintextPasswordがそのまま後続処理に使われていれば、セキュリティの観点からバグの疑念が生まれたり、発見に繋がり易くなるため属性値は付けてあげると親切。
4. 名前の長さを決める
「良い名前」を付けたとしても長さは重要。
弊社では「国内航空券」と「海外航空券」の購入するサービスを使っていますが、この名前がまあ長い。エンジニアの中では「国内航空券」を「DomesticAirline」、海外航空券を「InternasionalAirline」と英語に直して使われています。
両者には同じ仕様などもあるので、一つのクラスにまとめようとして「DomesticAndInternasionalAirline」といった長めの単語になることもしばしばあります。
ただ、弊社はphpのpsr１２準拠のコード制約をしており、これだとCIのLintチェックで弾かれてしまいます（変数名であれば20文字まで、クラス名であれば40文字まで）。こういう状況ではどうしたら良いのか。リーダブルコードでは以下のように説いています。
- スコープが小さければ短い名前でも良い
- 長い名前を"入力すること自体は"問題じゃない
- 単語の省略系を使う
- 不要な単語を投げ捨てる
スコープが小さければ短い名前でも良い
例えば、コードが数行しかない場合で長い名前をつけることは不要です。
上記のtmpの例が同じような意味を持ちます。
弊社で表すと、「DomesticAndInternasionalAirline」の頭文字だけ取ってdiaなどと略しても数行であれば認知できる範囲なので、特段問題ではないということ。
長い名前を"入力すること自体は"問題じゃない
「タイピングが面倒だから出来るだけ短い名前で！」というのは最近ではあまり理由になりません。基本的にIDEであれば補完機能がついてますし、VSCodeにも普通についています。
そのため、「面倒だから〜」ということ自体はあまり理由になっていないのです。エディタがなんとかしれくれるので。むしろ、情報が正確に伝わるのであれば、長い名前を使う事自体は問題ではない。と説いています。
頭文字や省略系を使う
先ほど使った例として以下のような感じですね。
「DomesticAndInternasionalAirline」の頭文字だけ取って
diaなどと略しても
出来るだけ単語は短くしつつ、相手に伝わればOKです。
例えば、月を表す英語で「December」をこれを「Dec」と略すことはグローバル的に見ても普通なことです。月日に関係する処理のなかで「Dec」と出てくれば「あ、12月にはこういう処理に変えるのね」といった感じで他のコードからも読み取れます。
頻繁に使われている略語は積極的に使い、文字数を短くするというのはテクニックとして使うべきです。
https://www.rarejob.com/englishlab/column/20190713/
まあ、名前の長さとかはDDDを意識した設計だった場合、ユビキタス言語を用いていれば混乱という混乱は起きないのかなあという印象。
不要な単語を投げ捨てる
例えば、ConvertToStringというメソッドがあるとします。
これの情報で一番必要な部分はどこか。
ToString ですね。
「Convert」という単語は変形するという意味を持ちますが、「To」という状態遷移を表す前置詞ということもあり、ToStringだけみても「あ、String型にキャストするのね」と分かります。
なのでConvertという単語は不要なので切り捨てても問題がないと。
参考：https://eikaiwa.weblio.jp/column/study/english_study_skills/preposition-to
5.名前のフォーマットで情報を伝える
今最近では嫌われ者？のjQueryですが、このjQueryの変数名を付ける際に暗黙の制約ありましたね。
-
let name: 通常の変数名; -
let $name： jQueryオブジェクトを取得した変数名：
これも実はテクニックの一つでコード上で$~と出てくれば一発でjQueryオブジェクトだと認識できます。このようにフォーマットを用いることで読み手に意識的に情報を伝える事ができます。
【リーダブルコード】第３章 誤解されない名前
2章では「情報を名前に詰め込む」でしたが、それに誤解を生む情報を埋め込んでしまわないように「他の意味と間違えられることはないだろうか？」何度も自問自答して考えよう。と説いています。
例えばfilterといった関数があるとする。
results = DB.all.filter("year <= 2020")
このresultsの結果に入るのは一体なんだろうか。
- 「year <= 2020」のオブジェクト？
- 「year <= 2020」ではないオブジェクト？
こういった誤解を招かないためにも、情報は明確にした方が良い。
選択されているのであればselectを、除外されているのであればexcludeなどの単語を使った方が適切。
- 限界値を含める時には
minとmaxを使用する - 範囲の指定をするときは
firstとlastを使用する - 包括・排他的範囲については
beginとendを使用する - ブール値の名前は
trueとfalseの意味を明確にする必要がある - ユーザーの期待に合わせた名前を付ける
- 複数の名前を検討する
限界値を含める的にはminとmaxを使用する
限界値と考えるとLimitという単語が思いつく場合がある。
このLimitという単語はコードで表そうとした時に、限界値を含むのか含まないのかという誤解を生んでしまう。そのためにあまり使わない方が良い。
つまり、「<」か「<=」で変わってきてしまう。
であれば最初からminとmaxで限界値を表してあげた方が誤解を生まなくて良い。
範囲の指定をするときはfirstとlastを使用する
range(start = 2, stop = 4)
startが2なのは分かり「2,3」は確実に範囲に入る。しかしstopの「４」は含むのか、含まないのかパッと分からない。lastなら包含している事が明確になる。
ただ、「正しく聞こえる」「意味として捉えられるのであれば」特段そこまで気にする必要性もないような感。
包括・排他的範囲についてはbeginとendを使用する
排他的範囲なので含まないとして考えると
「2020年12月1日の12時から、2020年12月25日の12時まで」との範囲として場合は以下のようにする
rangeApplicationTime('2020-12-01 12:00:00', '2020-12-25 12:00:00)
理由はこっちよりも見えやすい
rangeApplicationTime('2020-12-01 12:00:00', '2020-12-25 11:59:59)
使う場合はrangeApplicationTime(begin, end)
ブール値の名前はtrueとfalseの意味を明確にする必要がある
例えば、よくあるのはflgとする変数。
営業日かどうかを判定するのに、bussinessday_flgなどの変数名から情報が読み解けない。
0だったら？1だったら？、それ以外の数字があったら？処理分岐が面倒だし、その数字が正しい設定値なのかも不明。
ブールの値には「is, has, can, should」を使う事が一般的で解決する事が多い。
先ほどの例であれば、is_bussinessdayなどで代替できる。
またブール値には否定形を含めない方が紛らわしくなくて良い。
- ❌
disable_ssl = false - ⭕️
use_ssl = true
ユーザーの期待に合わせた名前を付ける
例えば、get***というメソッドの例。
カプセル化の概念からgetterの名前によく用いる。メンバ変数の取得に使われる軽量のメソッド名というのが暗黙の了解。そのため、このgetで始まるメソッドにコストがかかる処理を書いてはいけない。
もし、そこにDBにアクセスして、取得した値をコンバートして、計算して...などといった処理を内部で行っていればユーザーの期待に沿っていない。軽量かと思って入れた処理が、サービスに影響を及ぼすメソッドなんてことは珍しくない。
軽量なら軽量らしい振る舞いを、重量があるのであればそういった名前に変えてあげるか、軽量に分割する処理に変えてあげるべき（元も子もない）
第1章 - 第3章までのまとめ
第1章にも書いてあったことと同じように**「自分の書いたコードが他の人が読んで正しく意図を理解できる。」**というのを常に意識する必要がある。プログラミングに使う単語は時に曖昧な単語を含んでいる事が多いので、そこを気をつけるとお互いに幸せになれる。
個人的にはライブラリであったり、フレームワークの中にも色々混じっている気がする。
（まあライブラリであれば、ラッパークラスを用いて使いやすくしたりすることもあり）
って感じで第1章 - 第3章まで読んだ結果のアウトプットをざっと書きました。
残りはまだまだ書き途中なので暇つぶしに更新していきます。（ほんまかな）
所感もどき
まだまだ続きますが...以下はポエムなので読み飛ばして良いです。（嘘、暇なら読んで）
コードレビューで相手にこれってどういう意味って聞かれた瞬間に、それはきっと自分は分かりにくいコードを書いてしまっているのだなあと反省して活かすことを仕事中は考えています。
「誰やこのコード書いたの！！！？GitLensで犯人探したる！！！.......2ヶ月前のわいやんけ」
ってなることもしばしばなので、そういうこと自体も減らしていきたいなと。
これにはやっぱり訓練が必要だなと毎日感じています。（常に意識して、最短で最速で考えないとコードが書く時間が減ってしまう。）
ただ、「コードが分かりやすく書く」これには客観的評価が絶対的に必要で、自分的には分かりやすく書いたつもりのコードでも、他者から見てそうでもないって事もあるので、常に他者からの評価は必要です。なので、コードレビューする人は軽いコメントでも良いので「あ、これ分かりやすくて良きでした！」とかのコメントをしてあげた方が伸び率は格段に上がると思います。褒めて伸ばしましょ。
逆にコードの書き手は、コードが綺麗と評されいるコードを見ながら、レビューで殴られながら成長していくしかないかな。このプロジェクト、OSSのコードが綺麗とかあればコメントで教えていただけると嬉しいです。