自分がコードを書く時に気を付けていること
ふと思い立って、自分がコードを書く時に気を付けていること・心がけていることを振り返ってみた。
大きなくくりでいうと、こんなところ。
- 読みやすいこと (インデントや名前など、表面的なところ)
- 改修しやすいこと (ある意味での「読みやすい」こと)
- 冪等性・何度繰り返しても同じように動くこと
- 処理内容を追跡できるログを出力する
以下、それぞれの内容についての細かな話を書いてみる。
表面的な読みやすさとして心がけていること
例えばこんなところ。
- インデントを正しく付ける
- ケースを揃える
- 変数名・関数名などの品詞が適切である (英文として自然に読めるように)
「インデントをきちんと付けたり、名前を考えたりすると、作業速度が落ちるから、急いでいる時は適当にやる」という人がいるが、その認識は間違いだと断言する。そんな人は汚くてミスのあるコードを作ってしまった時に、読み解くコストが余計にかかり、正しいモノを提出できるまでに時間がかかることだろう。
まず、その程度のことで「作業速度が落ちる」ことの方が問題。キーボード操作が遅くてインデントごときに手間取ったり、頭が悪くて命名がヘタクソだったりして、そうした作業に時間がかかってたり負担を感じていたりすることの方を直さないといけない。平たくいえばバカだから遅ぇんだよ頭良くなれ。
自分はタイピング速度が速いからインデントを正しく付ける作業に時間がかからないし、命名も瞬時に妥当なモノが付けられる。その結果最初からキレイなコードが書けるから、もしバグがあってもすぐに見つけて修正できて、結果的に短時間で済む。
表面的なコーディングスタイルなんかが不揃いだと、中身を読み解く以前の段階のコードリーディングにかかるコストが増える。中身がすんなり入ってこなくて、勘違いしたりしやすくなる。そうしたノイズを減らして中身に注視してもらうために、表面的なところは特に驚き最小の原則を心がける。
「品詞」の意識も大事。変数は名詞、関数は動詞、ぐらいのことはまだいいが、目的語にあたる内容を関数名で示すか、それとも引数名で示すか。クラス名は主語にあたるので、クラスの分割粒度が正しくないと英文法として不自然になったりする。他にも、配列の変数は複数形で書き、For-Each で要素を取り出した時は単数形で書く、とか。
改修のしやすさとして心がけていること
例えばこんなところ。
- 変数名・関数名などから処理内容を読み取りやすくする
- 勝手に省略した単語を使ったりしない
- 影響範囲を調べやすくする
- 同じモノは同じ文字列で記述する
- 修正された時にバージョン管理ツールでの差分がキレイに出やすくする
自分が書いたコードが、いつか誰か他の人に直される時のことを考えてコードを書いている。コードを直すためにはまず中身を読んでもらうことになるので、それが何をしているコードなのか、読み取りやすくなるよう工夫している。
影響範囲を調べたりする時は、コード全体を grep したりすると思うので、grep でヒットしやすくなるよう、同じモノは同じ文字列で名前を付けてやる。getItem() と addData() とが同じ Book というリソースを扱っている、みたいな調べにくい命名を避ける。
また、修正された時に、Git で行単位の差分がキレイに見えることを狙って、処理ごとに行を分けて書いてみたりしている。複数の変換処理を経てデータを返却する関数を作ったとして、「追加要件によっては、この変換処理の間にさらなる変換処理を追加するかもしれない」と思ったら、その処理と処理の間を開けておいてやる。といった感じ。
冪等性を心がける
冪等性とは、同じ操作を何度繰り返しても同じ結果が得られる、ということ。
SQL なんかでいうと、INSERT 文を発行するコードは、そのまま2回目も実行すると、データが既に登録されているので、INSERT できずにエラーになる。コレを回避するとしたら、例えば UPSERT 文を使うようにするとか、SQL を投げるアプリ側でエラーハンドリングするなどして、その関数の実行結果としては、何度叩いても同じ状態にすることが望ましい。
シェルスクリプトでいうと、echo HOGE >> とリダイレクトで追記するような処理は、何も考えずに実装してしまうと、実行した回数だけ同じ文字列が追記されることになる。冪等性を考慮するのであれば、その文字列を追記する必要があるかどうかを事前にチェックする必要があるだろう。
REST API における PUT メソッドなどでも、このようなことが言われたりする。Ansible という構成管理ツールは冪等性を保つために、「実行しなくて良い処理は再実行しない」ように動くことで、結果的に設定される内容が同一に保たれる。
追跡可能なログを出力する
契約プログラミングの考え方でいくと、ある関数は、
- 事前条件：処理を始める前に、前提条件をチェックする
- 事後条件：処理をした後に、あるべき結果になっているかチェックする
- 普遍条件：処理の前後で変化してはいけない部分をチェックする
という、3つの条件が守られていることを、アサーション等で確認する必要がある。
ログ設計についても、この3つの情報を出力するようにしておくと、問い合わせや障害が発生した時に、その処理をトレースしやすくなる。
シェルスクリプトでいうと、例えば、あるファイルの文字列を置換する、1つの処理があったとする。
sed -i 's/hoge/fuga/' ~/example.txt
sed は置換対象がなくても、終了コードは 0 で、思ったとおりに置換できたのかどうかが一見分かりにくい。
契約プログラミングとして強制力を持たせるなら、置換後の文字列が存在するかどうかで grep し、なければエラーとする、といった形で対応する。これはこれで実装するワケだが、同時に、「処理前はどうだったか」「処理後はどうなったか」というログも残しておくと良いだろう。
# 事前チェック : 「hoge」が含まれているかどうか
grep 'hoge' ~/example.txt
# 置換処理
sed -i 's/hoge/fuga/' ~/example.txt
# 事後チェック : 「hoge」が含まれていないこと
grep 'hoge' ~/example.txt
# 事後チェック : 置換した「fuga」が含まれていること
grep 'fuga' ~/example.txt
前後に追加した grep コマンドによって、事前・事後の状態がログに残ることになる。コレがあれば、この当時はどういう風に何が起こったかを追跡できるだろう。
全ての箇所に実装するのは大仰だと感じるかもしれない。チェックのための処理も、それだけ処理コストがかかるので、過剰なロギングは性能低下を招く恐れすらある。だから「チェック・ログ出力すべき箇所」と「しなくてもよさそうな箇所」を見極める必要はあるが、迷った時はログ出力しておいた方が、追跡しやすいだろう。
処理内容を正確にトレースできないと、障害調査に膨大な時間がかかり、確証が持てないと調査結果の信頼性も落ちることになる。そうした時間的コストと比べたら、ちょっとしたチェック処理を実装するコストと、それを都度実施するマシンの処理コストをとった方が良い場合は多いだろう。
以上
自分が当たり前にやっていることでも全然できない人が多いので、書いてみた次第。実装者である自分を信用しないからこそ、堅牢で信頼できるコードが書けるようになる。
…やっぱりリーダブルコードに行き着く。この本を読んだことがあるか否かで、ハッキリ別れる。
