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

プロジェクト推進能力

コミュニケーション能力のところでも思ったのですが、この言葉は本当にいろいろな意味で使われますので、各自定義が必要かと思います。 今回はコミュニケーション能力とは違うのですが、プロジェクトを推進していく …

no image

コレクションの頻出処理に関して

PHPでコレクションを使っていますが、慣れると本当に便利ですね・・・まあforeachとかでグリグリやってもいいのですが、無駄にコードが長くなります。 自分がコレクションでよく使う再頻出のメソッドなど …

no image

スクラム開発をやってみて

アジャイルの定義に関してはエンジニアによって色々なものがあると思われますが、自分の場合一番綺麗に説明されているな・・・と感じたのは以下の記事でも紹介した書籍でした。 アジャイル開発について 現場ではた …

no image

制御フローについて

リーダブルコード 7章。制御フロー(if文などの条件分岐)について ここらへんは個人個人癖がついているとおもいますが、確かに読みやすい、読みにくいというのはあるのでなるべく汎用性のある規則を身につけた …

no image

API仕様書に関する注意事項

API仕様書を作っていて、基本的な点についてのまとめ コードと連動できれば理想(現実的には設定ファイルをJSONかYamlで作るぐらいが限界だと思う) 型のチェック、必須チェック、桁数チェック、日付の …

アーカイブ