背景
新人開発者の頃、幸運にもコードレビューを受けられる環境で開発を始めました。最初は、レビューの基準が何なのかもよく分かっていませんでした。単に機能が正常に動作すれば開発は終わりだと思っており、レビューで指摘される内容も、最初は些細なことに感じていました。
importの順番や変数名のように、機能とは直接関係がないように見える部分についても、次のような質問を受けることがありました。
-
「この状態値は本当に必要ですか?」
-
「watchで処理していますが、イベントで処理するほうが適切ではありませんか?」
-
「このコンポーネントは責任を持ちすぎていませんか?」
-
「null、undefined、空の配列を適切に処理できていますか?」
最初は、一つひとつ修正するのが面倒に感じることもありました。しかし、チーム単位での開発や運用中のサービスを直接経験する中で、少しずつ考え方が変わっていきました。「今、機能が動作しているか?」で終わるのではなく、「このコードを別の人が再び修正することになったら、簡単に理解できるだろうか?」と考えるようになりました。
運用環境では、「後で問題が起きたら、別の開発者が直してくれるだろう」と考えて作業するのは難しいものです。チームで開発していても、自分が書いたコードを別の人が再び修正することもありますし、数か月後に自分がそのコードを見直すことになる可能性もあるからです。
こうした経験を繰り返すうちに、過去のコードレビューで確認していた項目を、開発の過程でも自然に一度ずつ確認するようになりました。
もちろん、今でもすべてのコードを丁寧に確認するのは現実的に困難です。特に開発スケジュールが厳しい状況では、なおさらです。そこで私が言う「セルフコードレビュー」は、精密な分析というよりも、素早く開発しながら見落としやすい問題をもう一度確認する、最低限の検証プロセスに近いものです。
1. セルフコードレビューが必要な理由
素早く開発する状況では、機能を作り、正常に動作することを確認した後、次の作業に移りがちです。開発速度だけを考えれば、効率的な方法かもしれません。しかし、開発が速いほど、コードを客観的に見る機会は減少します。コードを書いている時点では全体の文脈を把握しているため、不要なコードや曖昧な構造を自然に見過ごしやすくなります。
2. 機能が動作したからといってレビューが終わったわけではない
画面が正常に表示され、ボタンを押したときに期待した結果が出れば、開発が完了したように見えます。しかし、実際のサービスには正常な状況(Happy Path)だけが存在するわけではありません。APIリクエスト一つを見ても、さまざまな状況を考慮する必要があります。
-
正常なレスポンス → データ表示
-
API失敗 → エラー処理
-
レスポンスデータなし → Empty状態
-
undefined / null → データの存在確認および安全な処理
-
連続クリック → 重複リクエストや状態の不整合を防止
セルフレビューでは、単に「正常に動作するか?」で終わらせず、「異常な状況でもどのように動作するか?」も併せて確認します。
3. 1回目のレビュー:不要なコードを見つける
最初に見る部分は、意外にもシンプルです。「このコードは本当に必要か?」
① 使用していない変数とimport
テストのために残したconsole.log()や、もはや使っていない変数、importされたモジュールがないかを確認します。こうした部分は、ESLintのような静的解析ツールの助けを借りて自動化するのがよいでしょう。
② 重複コード
複数の画面で同じAPI呼び出しとエラー処理を繰り返している場合、共通化する必要がないかを検討します。ただし、無条件に共通化することが正解とは限りません。「このコードを共通化したとき、実際に保守しやすくなるか?」を基準に判断します。
③ 過剰な状態値
// ❌ AS-IS
const dataList = ref([]);
const isEmpty = ref(false);
const hasData = ref(false);
// ⭕ TO-BE
const dataList = ref([]);
const isEmpty = computed(() => dataList.value.length === 0);한 상태값
上記のように、dataListさえあれば計算できるisEmptyやhasDataのような値を、別の状態(ref)として管理する必要がないか、まず検討できます。
状態を個別に管理する場合、データを更新するたびにisEmptyやhasDataの値も手動で更新する必要があります。状態更新ロジックの一つでも漏れると、実際にはデータが存在するのに、画面には「データがありません」と表示されるバグが発生します。computedを活用して元のデータに依存して動作するようにすれば、状態値を手動で同期しなければならない部分を減らし、開発者が状態の更新を漏らす可能性も下げられます。
4. 2回目のレビュー:コンポーネントの責任を確認する
最初は単純だったコンポーネントも、開発が進むにつれて、ユーザー検索、認証、フォーム検証、モーダル、ページングなど、数多くの責任を抱えるようになります。
大きなコンポーネントを見たら、機能ごとに領域を分けて、独立して分離できるかを検討します。ただし、コードが長いという理由だけで無作為に分割すると、ファイルを探すコストだけが増えてしまいます。重要なのは「小さくすること」ではなく、「責任を明確にすること」です。
PropsとEmits: 親があまりにも多くの状態を直接制御していないか、イベント名を見るだけでどのような動作なのか推測できるかを確認します。
5. 3回目のレビュー:状態と例外状況を確認する
個人的には、セルフレビューで最も重要視しています。画面は正常な状況よりも、例外的な状況で簡単に壊れます。
①LoadingとErrorの状態
-
データの読み込み中に、ユーザーに適切な状態が表示されているか?
-
APIが失敗したとき、ユーザーが次の行動を取れる状態として処理されているか?
実際にあるプロジェクトのコードレビューを行った際、認証プロセスで特定のエラーが発生したときの例外処理が漏れており、画面全体が真っ白になって、その後の手続きを進められない問題を発見したことがあります。正常な認証フローでは問題がなかったため、正常なケースだけを確認していたら見落としやすいエラーでした。この経験以降、API呼び出しのコードを見るときは、成功したときの結果だけでなく、「ここで失敗したら、ユーザーはどのような画面を見ることになるのか?」も併せて確認するようになりました。
② EmptyとNull / Undefined
データが空の配列([])である状況(Empty State)と、APIが失敗した状況は異なります。また、user.profile.nameのような参照でprofileが存在しない可能性を考慮しないと、ランタイムエラーによって画面が正常にレンダリングされないことがあります。
6. 4回目のレビュー:非同期処理とAPI呼び出しを確認する
重複呼び出し: ページへの प्रवेश時、watchの実行時、ユーザーイベントの発生時に、同じAPIを不必要に呼び出していないかを確認します。
非同期処理の順序(Race Condition): ユーザーが素早く2回アクションを実行したとき、後からリクエストしたデータが先に到着し、以前のリクエストのデータが遅れて到着することで、最新の状態を上書きする可能性がないかを確認します。
7. 5つ目のレビュー:保守性の観点から見直す
最後に、初めてコードを見る人の視点で整理します。
命名とHTMLセマンティクス:const tempではなく、役割が分かる名前を付けているか、クリックイベントが必要なボタンに<div>ではなく<button>を使用しているかを確認します。
コメントと使用していないコード:使用していないコードはコメントとして残しておくよりも、整理するようにしています。過去のコードはGitの変更履歴で確認できます。コメントが必要な場合は、コードだけを見てもすぐには理解しにくい理由や、特に注意すべき内容を残します。
構造の一貫性:import、props、state、methodsなどの配置や記述方法が、プロジェクトの既存コードと大きく異なっていないか確認します。ただし、既存の方法に無条件で従うのではなく、現在のコードにより適した方法がないかも併せて検討します。
プロジェクトにすでに共通のAPI処理方法やユーティリティ、UIコンポーネントがある場合は、新しい方法を作る前に既存の実装を先に確認します。
既存の方法をそのまま使用することもできますが、状況によっては新しい方法を選択することもあります。重要なのは、既存の方法がなぜ使われているのかを先に確認し、現在のコードにどの方法がより適切かを判断することです。
たとえば、API呼び出しとエラー処理がすでに共通化されている場合は、まず既存の方法を活用できるか確認します。反対に、既存の方法が特定の状況でしか動作しない、または改善が必要な場合は、新しい方法を適用することも検討できます。
8. 実際の変更内容を基準にセルフレビューする
コードを書いている途中にこれらすべてをチェックすると、かえって開発速度が遅くなります。私は、機能の実装を優先して終えた後、Git diffやPRの変更内容のように、実際に修正されたコードだけを確認できる画面で改めて見直す方法を好みます。
書いているときは「どう実装しよう?」に集中していたなら、レビューするときは「自分が初めて見るコードだとして、理解できるだろうか?」へと視点を切り替えます。
【最低限のセルフレビュー・チェックリスト】
-機能
-
正常な入力に対して、意図どおりに動作するか?
-
Loading / Error / Emptyの状態を処理しているか?
-
null / undefinedの可能性を確認したか?
-
既存の機能に影響を与えないか?
-コード
-
使用していない変数 / import / console.logがないか?
-
不要なstate値を作成していないか?
-
重複したロジックを確認したか?
-
コンポーネントの責任が大きくなりすぎていないか?
-API / 非同期
-
同じAPIが不要に重複して呼び出されていないか?
-
APIエラーの処理は一貫しているか?
-
連続したリクエストによって状態が混乱する可能性はないか?
-保守性
-
変数と関数の名前だけを見て役割を理解できるか?
-
使用していないコードや古いコメントがないか?
-
既存のパターンと異なる方法を使用した場合、その理由があるか?
-
初めて見る開発者がコードを追えるか?
9. 実務でのレビュー経験と現実的な妥協
新人時代にレビューを受けながら身につけたことは、その後、コードレビューを直接担当するようになったときにも役立ちました。単にコードの問題を見つけるだけでなく、その後の修正や保守にどのような影響を与えるかも併せて見るようになりました。実際にこのような観点でコードを確認することで、開発過程では見落としやすいエラーを発見し、修正することもありました。
しかし、すべてのプロジェクトで完璧な基準を当てはめることはできません。スケジュールが厳しいプロジェクトを支援したときのことです。私自身も機能開発を担当していたため、既存のコードをすべて整理する現実的な余裕はありませんでした。このときは優先順位を決め、必要な部分から改善しました。
-
機能に影響を与えるクリティカルな問題
-
保守の直接的な妨げになる部分
-
共通化したときのメリットが明確な部分
-
単純なコードスタイルの整理(consoleの削除、コメントの整理など、最小限にとどめる)
スケジュールが厳しいという理由ですべての問題を無視すると、技術的負債となって返ってきますが、一度に完璧を目指してスケジュールを逃すことも問題です。そのため、状況に応じて優先順位を決め、必要な部分から改善するようにしています。
まとめ
セルフコードレビューは、必ずしも大げさなアーキテクチャレビューである必要はありません。特に迅速な開発が必要な状況であれば、なおさらです。私が考えるセルフコードレビューとは、結局のところ「機能の実装が終わったコードを、もう一度疑って見てみること」です。
新人時代は、コードレビューで指摘された内容を一つずつ修正しながら学びました。今でも、そのときのレビューで受けた質問を、似たような状況で一度は思い出すようにしています。
迅速に開発しなければならない状況であれば、すべての部分を完璧に検討するよりも、5分だけでも見落としがないかをもう一度確認することから始めてもよいでしょう。
Code_Latte