skillup

技術ブログ

プログラミング全般 プロジェクト管理

コードレビュー時のポイント

投稿日:2021年12月5日 更新日:

コードレビュー(仕様的な点ではなくて規約的なところのチェック)などで気をつけることなど。

ポイントとしては検知ツールで確認するコストを減らすことが一番大事(カスタマイズの方法について調べておく)、あるいはコミット時に静的解析ツールが走って、ルールを守らないとコミットできないなど。

人チェックをしないのがコツだが、する場合、10以上のルールを同時にチェックすると漏れるため、メソッド単体ぐらいに切ってゆっくり見ていくほうが良いでしょう。普段からチェックし、チェックリストを作り、一気にチェックをしないことが大切だと思います。

規約に関する確認

ありがちなルール

文字のキャメル、スネークの統一、その他ルール

おそらくツールで検知できるのでこれで回避。稀にerrmsg(errMsg)のように検知できにくいものもあるので注意。

スコープの長さと変数名を対応させるなども、コードを書いていて、癖がつく部分もあるが人チェックが必要。

if文やfor文のスペースや括弧の位置

癖をつけておけば大丈夫(目が違和感を覚える)だと思うが、100%になることはなかなか難しいので、ツールでチェックできるようにしましょう。

コメントの書き方

Document作成ツールでチェックするのが一番わかりやすいが、内容のチェックまではおそらく人チェックがいると思います。

入れる場所などは完全なシステム的なチェックは難しいので人チェックで。

行の長さ、メソッドの長さ、クラスの長さチェック

普段からコードを書いて癖づけするのが一番(ここだけに限りませんが・・・)

1行あたりの場合はチェックがつくことが多い(100文字以下など)ですが、メソッド、クラスの長さなどはチェックができないことが多いでしょう。

この点は分割することを癖にするのが一番良いく、規約云々ではなく、このほうが書きやすくなることが多いです。

引数チェック

5以上を超えると見にくくなります。規約もそうですが、こちらも保守性が高いコードを書いておけば必然的に身につくはずです。

特定メソッドの推奨

一番見落としがち。例えばJavaではStringBuilderで文字を追加する、Listの定義ではArrayListやHashListを使わないなど。同時に複数チェックを入れると見逃しやすいので、この点だけに注意してチェックする。

言語自体への慣れもそうですし、普段から模範的なコードを書いておく必要があると思います。

コードの内容に関するルール

命名

  • 名称が適切か、冗長すぎないか
  • メソッドに関して規則があるか(get、setなどの頻出のものを多用しすぎていないか)

責務の分離

  • コードのクラス、メソッドの役割が適切か
  • 適切に責務に分離されていれば、Controllerでもない限り、メソッドがながくなることはないはず
  • 各クラス分けが適切か(Controller、Service、Modelなど)
  • Controller、Service、Modelの仕事がそれぞれ1つずつか(色々な仕事をさせていないか)

可読性

  • 引数が多すぎないか数は適切か
  • 1つのメソッドで1つの責務になっているか
  • 引数に必要以上に配列を使っていないか(ブラックボックスになりがちなので、少なければ直接引数を。多ければそれ用のObjectを。)
  • 定数などを定義Enumなどで置き換えているか(他人が見たときにコードがわかりやすいか)
  • 配列を使いすぎていないか(役割を持ったオブジェクトに置き換える、分けるなど)

型のチェック

  • DBに入れる場合に型チェックしたものを使っているか。setterの方がfillなどで一気に値を入れるよりも値がしっかり担保される
  • nullのある場合、可能性を常に考慮しているか
  • getter,setterなどでこれらの値の保証ができているか
  • 不十分なメソッドや条件判定(3重イコールではなく、2重イコールを使っていないか。またemptyなどを使っていないか。正しくはis_nullなど。)

抽象化

  • コードが適切に抽象化されているか(共通のものが多い場合は親を作り、親子関係ができているか、あるいは親子の関係が適切か)

保証された値を使っていないか

  • requestパラメータはバリデーションを通過したもののみを使う
  • 何らかのfilterを通過した値を使っているか

負荷

  • 大量のデータを一括で取得していないか(chunkなどで小出しにとる、分割するなど)
  • 画面の場合はデータ量を考えて設計されているか
  • レスポンス時間が考慮されているか
  • そもそも使用するミドルウェアが正しいか(DBでなくNoSQLを使うなど)

その他

  • フレームワークの標準の機能などを使っているか
  • テストコードが適切か
  • プロジェクト内のコーディングの各種ルールを守っているか

-プログラミング全般, プロジェクト管理
-

執筆者:


comment

メールアドレスが公開されることはありません。 * が付いている欄は必須項目です

関連記事

no image

プロジェクトマネジメントについて

以前、主に座学で勉強したことをベースにプロジェクトマネジメントについて で書いたのですが、ここ2年ぐらい比較的大きめのプロジェクトを色々と経験させてもらったのでそこで得たことなどを。 Contents …

no image

ミスを少なくする工夫について

プログラマであればだれもが「いかにバグを少なくするか」に腐心すると思います。 ところが、人間がある以上、バグ(ミス)は絶対にゼロにはなりません。バグ云々以前に、「人間はもともとミスをする生き物だ」とい …

no image

設計業務での改善点など

比較的大規模なプロジェクトの設計段階で思ったことなど。会議など頻繁に開かれると思いますが、聞いているだけだったり、各人が色々なことを喋って内容がまとまっていなかったり・・結局何をやりたいかわからなくな …

no image

小〜中規模程度のWEBアプリ作成で気をつけるべきこと

初見の処理系(ライブラリ操作)などは休日などで最小パターンを確認しておくこと。実務で何時間も悩むと非常にストレスがたまる テーブル設計命。あとで終えるようにトレースができるような値を入れておくこと。 …

no image

レビューについて

以前、コードレビューそのもののポイントについてはコードレビュー時のポイントでまとめましたが、レビュー自体の体制の注意点やそこでの気づきなど。 Contents1 目的2 定型的なものはチェックリストを …

アーカイブ