こんにちは。アプリエンジニアの大橋です。
今はそれほどではないのですが、少し前まで仕事の大半をコードレビューに費やしていた時期がありました。
どちらかというと自分で実装する方が断然好きですが、
それはさておき、いろんなコードを見るなかで、
「このコードの書き方だと、正常に動作してはいるけど、後々不具合を生みやすそうだなぁ」
と感じることがあったりするので、
そういうコードのパターンをいくつか取り上げさせてもらおうかなと思います。
自分自身も気を付けたいという意味も多分に含んで。
というわけで、さっそくひとつめ。
変数名や関数名が処理内容と違う
基本的なことではありますが、たまにやってしまいます。
コードを読むとき、変数名を見て「こういうことに使ってる変数かな？」と想像して読んだりしますが、
実際は違った使われ方がされてると、コードの理解の妨げになりますし、
誤解したまま別の人がその変数を使ったりすると不具合の原因になってしまいます。
関数名やクラス名もまた然り。
ちゃんと処理内容にあった変数名や関数名を付けましょう！！
あと余談ですが、自分が実装するとき、変数名や関数名の名前に結構悩みます。
的確な名前を付けたいと思うのですが、英語が得意というわけでもないので、
なかなかこれだっていう名前が付けられないときがたまにあって、
そういうときは大抵、辞書を引いてニュアンスとかを考えだしたりして、深みにハマります……
英語が得意じゃないプログラマーは、英語圏のプログラマーより生産性がかなり落ちるんじゃないかと
常々思うところです……
あと、これに関連してですが、
耳慣れない単語を使うと後々困ります……
単語の意味が合っているからといって耳慣れない単語を使うと、
- 他の人がコードを把握しようとしたとき単語の意味が分からないので辞書を引かなければいけない
- 読み方を忘れるので会話するとき困る
ので、わかりやすい名前を付けましょう！！
そしてふたつめです。
public関数に暗黙の使用条件がある
ついついやってしまうのですが、気をつけたいなと思うことです。
例えば
「ある関数Aを実行するときは、その前に別の関数Bを実行しておく必要がある」
といったことがあると思います。
もし前提条件の関数Bを実行せずに関数Aを実行してしまった場合の
エラーハンドリングがちゃんとできていれば大丈夫かとは思うのですが、
エラーハンドリングもされず、その関数やクラスのコメントにも関数Bをあらかじめ実行しておく必要があることが
書かれていない（＝暗黙の条件）状態だと、
後から別の人がその関数を使ったり修正したときに不具合が発生する危険が増しますし、
コードを把握するための時間もかかってしまいます。
実装した本人でも忘れてしまいますし。
あまり関数を使うための前提条件をつくらない方がベストな気がしますが、
やむおえない場合はきちんとコメントに書いておきたいところです。
ということでみっつめ。
関係なさそうなところで影響し合っている
「ある関数の処理内容を変更したら、実は他の場所でその関数の処理内容に依存した実装があり、不具合が発生した」
というようなことがよくあるような気がします。
特に、あまり関係なさそうな処理同士が依存していると、
後から関連処理を修正するときに見逃しやすく、処理を把握するのも大変。
処理の依存関係はできるだけ小さい範囲でまとめたいところです。
まぁ、テストを書け、ということかもしれませんが、
依存関係が散らかってないコードを書ける人は尊敬します。
そしてラスト。
難解すぎて何をやってるかわからない
凝ったコードでも他の人が理解しやすいコードはいいのですが、
なにをやっているか理解しづらい難解なコードは避けたいところです。
他の人が読み解くのに時間がかかりますし、
完全に把握できていない状態で変更を加えられたり、コピペして使われたりすると危険です。
コードを短くするために凝ったコードを書くより、
ある程度長くなっても理解しやすいコードのほうが良いんじゃないかなと自分は思います。
ムダに冗長なコードは避けたいですが。
最後に
この記事を書いていて思いましたが、
不具合を生みにくいコードを書くことで、「不具合を生まない」ということだけでなく、
後々の生産性の向上にもなるような気がしました。
自分自身も、
ついついめんどくさがって、あまり後々のことを考えずに簡単に実装できるコードを書いてしまいますが、
後々のことを考えて、できるだけ不具合が生まれにくいコードを書いていきたいものです。