コードレビュー(仕様的な点ではなくて規約的なところのチェック)などで気をつけることなど。
ポイントとしては検知ツールで確認するコストを減らすことが一番大事(カスタマイズの方法について調べておく)、あるいはコミット時に静的解析ツールが走って、ルールを守らないとコミットできないなど。
人チェックをしないのがコツだが、する場合、10以上のルールを同時にチェックすると漏れるため、メソッド単体ぐらいに切ってゆっくり見ていくほうが良いでしょう。普段からチェックし、チェックリストを作り、一気にチェックをしないことが大切だと思います。
Contents
規約に関する確認
ありがちなルール
文字のキャメル、スネークの統一、その他ルール
おそらくツールで検知できるのでこれで回避。稀にerrmsg(errMsg)のように検知できにくいものもあるので注意。
スコープの長さと変数名を対応させるなども、コードを書いていて、癖がつく部分もあるが人チェックが必要。
if文やfor文のスペースや括弧の位置
癖をつけておけば大丈夫(目が違和感を覚える)だと思うが、100%になることはなかなか難しいので、ツールでチェックできるようにしましょう。
コメントの書き方
Document作成ツールでチェックするのが一番わかりやすいが、内容のチェックまではおそらく人チェックがいると思います。
入れる場所などは完全なシステム的なチェックは難しいので人チェックで。
行の長さ、メソッドの長さ、クラスの長さチェック
普段からコードを書いて癖づけするのが一番(ここだけに限りませんが・・・)
１行あたりの場合はチェックがつくことが多い(100文字以下など)ですが、メソッド、クラスの長さなどはチェックができないことが多いでしょう。
この点は分割することを癖にするのが一番良いく、規約云々ではなく、このほうが書きやすくなることが多いです。
引数チェック
５以上を超えると見にくくなります。規約もそうですが、こちらも保守性が高いコードを書いておけば必然的に身につくはずです。
特定メソッドの推奨
一番見落としがち。例えばJavaではStringBuilderで文字を追加する、Listの定義ではArrayListやHashListを使わないなど。同時に複数チェックを入れると見逃しやすいので、この点だけに注意してチェックする。
言語自体への慣れもそうですし、普段から模範的なコードを書いておく必要があると思います。
コードの内容に関するルール
命名
- 名称が適切か、冗長すぎないか
- メソッドに関して規則があるか(get、setなどの頻出のものを多用しすぎていないか)
責務の分離
- コードのクラス、メソッドの役割が適切か
- 適切に責務に分離されていれば、Controllerでもない限り、メソッドがながくなることはないはず
- 各クラス分けが適切か(Controller、Service、Modelなど)
- Controller、Service、Modelの仕事がそれぞれ1つずつか(色々な仕事をさせていないか)
可読性
- 引数が多すぎないか数は適切か
- １つのメソッドで１つの責務になっているか
- 引数に必要以上に配列を使っていないか(ブラックボックスになりがちなので、少なければ直接引数を。多ければそれ用のObjectを。)
- 定数などを定義Enumなどで置き換えているか(他人が見たときにコードがわかりやすいか)
- 配列を使いすぎていないか(役割を持ったオブジェクトに置き換える、分けるなど)
型のチェック
- DBに入れる場合に型チェックしたものを使っているか。setterの方がfillなどで一気に値を入れるよりも値がしっかり担保される
- nullのある場合、可能性を常に考慮しているか
- getter,setterなどでこれらの値の保証ができているか
- 不十分なメソッドや条件判定(３重イコールではなく、２重イコールを使っていないか。またemptyなどを使っていないか。正しくはis_nullなど。)
抽象化
- コードが適切に抽象化されているか(共通のものが多い場合は親を作り、親子関係ができているか、あるいは親子の関係が適切か)
保証された値を使っていないか
- requestパラメータはバリデーションを通過したもののみを使う
- 何らかのfilterを通過した値を使っているか
負荷
- 大量のデータを一括で取得していないか(chunkなどで小出しにとる、分割するなど)
- 画面の場合はデータ量を考えて設計されているか
- レスポンス時間が考慮されているか
- そもそも使用するミドルウェアが正しいか(DBでなくNoSQLを使うなど)
その他
- フレームワークの標準の機能などを使っているか
- テストコードが適切か
- プロジェクト内のコーディングの各種ルールを守っているか
