Skip to content

CodeReview.md

Latest commit

 

History

History
133 lines (123 loc) · 6.74 KB

CodeReview.md

File metadata and controls

133 lines (123 loc) · 6.74 KB

コードレビュー

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

  • 規約

    • 規則的にチェックできるものなので、レビューで人がチェックしないようにする
      • PHP_CodeSniffer(PSR-2)
    • 基本はエディタでチェックを行う
    • CICDでもチェックを行う(規約をやぶっている場合はcommitできないなど)

  • コメント

    • どれくらいの情報を詰めるかはプロジェクトルール遵守を
    • あえてしない・・・という選択肢もあり→その分、命名に注力すべき
    • ドキュメント生成や構文チェックにつなげられることも・・・

  • クラス、メソッド、引数、インデントの数、横幅の長さ

    • メソッド 30行以内
    • クラス 100行以内
    • 引数 5つ以内
    • インデント 3段階までがMAX
    • ポイントとしては責務の分離ができているかないか

  • 特定メソッドの推奨

    • パフォーマンスの向上など
    • stringは参照系、StringBuilderは更新系
    // Stringを使用した例
String str = "Hello";
str += " World"; // 新しいStringオブジェクトが生成される

// StringBuilderを使用した例
StringBuilder sb = new StringBuilder("Hello");
sb.append(" World"); // 既存のStringBuilderオブジェクトが変更される
String result = sb.toString(); // StringBuilderをStringに変換

  • 命名

    • 役割分担ができているか、必要な情報が込められているか
    • なるべくシンプルかつ統一性のある命名にする
    • 品詞の統一
    • 汎用的になりすぎない
    • メソッドに関して重複をしない

  • 責務の分離が保たれているか https://zenn.dev/mpyw/articles/ce7d09eb6d8117

    • Requests: ユーザーやクライアントからの入力データを表すオブジェクトを格納します。バリデーションやサニタイズなど、リクエストデータの前処理もここで行います。
    • Controller: リクエストを受け取り、適切なサービスやユースケースを呼び出してレスポンスを生成する役割を持ちます。ユーザーインターフェースとの仲介役です。
    • Service: ビジネスロジックを実装するためのクラスや関数を格納します。複数のコントローラから再利用されることが多く、アプリケーションの主要な機能を担います。
    • Infra (Infrastructure): データベース接続や外部サービスとの連携など、インフラストラクチャ関連のコードを含みます。リポジトリパターンやAPIクライアントなどがここに配置されます。
    • ValueObject: 一度作成されるとその状態が変更されないオブジェクトを格納します。ドメイン駆動設計（DDD）で使用され、特定の属性や値を持つオブジェクトです。
    • Model: データベースのテーブルに対応するクラスを格納します。データの保存、取得、更新、削除を行うメソッドを含みます。
    • ViewModel: ビュー（UI）に表示するためのデータや状態を管理します。コントローラから受け取ったデータを整形し、ビューに渡す役割を持ちます。
    • UseCase: アプリケーションの特定のユースケースやシナリオを実装するクラスや関数を格納します。ユーザーが実行する特定のアクションに関連するビジネスロジックを含みます。

  • 責務の分離→テストしやすさにつながる

  • 可読性

    // NG
$userData = [
    'name' => 'Alice',
    'email' => 'alice@example.com',
    'status' => 'active'
];

$user = new User($userData);

//OK
$userData = new UserData('Alice', 'alice@example.com', 'active');
$user = new User($userData);
    • まとめすぎない
    // NG
function resist($person_array);

//OK
function resist(
    $name,
    $age,
    $email,
    $address,
    $tel
)

  • 型のチェックによる値の担保

    • empty禁止(0,false,null,空文字,空配列が比較できてしまう)、二重イコール禁止(数字と文字の比較ができてしまう)
   // NG
   $userData = [
　　　'gender' => '男',
     'email' => 'yamada@test.com',
     'last_name' => '山田',
     'first_name' => '太郎',
     'tel' => '08012345678',
   ];
   $user = new App\Models\User();
   $user->fill($userData);

   // OK
   $user = new App\Models\User();
   $user->setGender('男');
   $user->setEmail('yamada@test.com');
   $user->setLastName('山田');
   $user->setFirstName('太郎');
   $user->setTel('08012345678');
   $user->save();
  • 負荷
    • chunk、cursorなどを使用してメモリ消費量を抑える
  • エラー処理
    • 適切なエラーハンドリング
      • トランザクションスコープ
      • 業務系のエラーか500系のエラーか
      • 後続処理への影響

レビューを円滑にすすめるために

  • レビュアーとレビュイーの振り分け
    • 全員がレビュアーかつレビューイ
  • 細かく分ける
    • とにかく粒度を細かく分けて、滞留させないように・・5ファイル以内、数分で内容が検知できることが望ましい
    • PRのテンプレートの充実など(情報がすぐにわかるように)
    • 1,2時間以内に検知できるのがベスト
  • 指摘に関して重さを分ける(must:必ず直す、should:直すべし、better:直した方が良い、want:直してほしい)
    • 数段階に分けて、必ず対応すべきものと、できれば直したほうがいいもの、個人的なものにわける
  • 類型化とノウハウの共有
    • コードレビューのノウハウの共有
  • リスペクトのコミニケーション
    • 当たり前だが、超重要
  • 神学論争的なものは責任者が決める
    • 決着がつかないものは責任者を立てて、その責任者が決める
  • 生成AIの活用例
    • 機械的にチェックされるので、既存のシステムへの適用が難しいかも
    • 細かすぎて、プロジェクト進捗を妨げるものが多いかも・・