はじめに

本記事では、個人的にプルリクエストのレビューで指摘されがちなケースとその対策について記載します。
指摘が中々減らずに困っている、プルリクエストを見る際にどこを見ればいいかあまり分からないといった課題の解消のお役に立てれば幸いです。

スペルミス

これはあるあるかと思います。
自分で書いている時にはなかなか気づかず、レビューで指摘されて気まずくなることが結構ありました。

対策

VSCodeでスペルチェックをしてくれるプラグイン（Code Spell Checker）を導入することで激減させることができました。

スペルを間違えると即座にエディタ側で指摘してくれるので、チームで導入しておくとレビュワーの負担軽減に繋がるのでとてもオススメです。

変数・関数・スタイルなどの命名

これも個人的にはよく指摘されます。
抽象的だったり、命名から役割が推測できないものはコードの可読性を下げるため良くありません。

/** 画面表示用 */
const objectA = { ...etc }         // x
const displayContent = { ...etc }  // ◯

対策

私のチームではよく使う単語や機能についてドキュメント化し、そこを参照して命名するようにしています。

チーム全体で認識を合わせることができれば実装中に迷うことが少なくなり、実装速度の向上にも繋がるのでこれもオススメです。

コーディング規約違反

プロジェクト毎にコーディングルールや暗黙の了解があったり...ということはよくある事かも知れません。

身体に染み付いたものであれば特に問題はないですが、むしろ癖で書いてしまっている記述がルールに違反していたりすると何度も指摘されてしまいがちです。

対策

機械的に弾けるものはeslint系で弾いてしまいましょう。
レビュワーの負担を減らしつつ、コード品質を向上させることができるので治安がとても良くなります。

また、定期的にルールの追加・見直しを行うことが出来れば尚良いかと思います。

機械的に弾けないものに関しては、コーディング規約をドキュメント化して周知を行い、プルリクエスト毎にセルフレビューを徹底しています。(かなり見落としがちですが...)

コーディング規約について理解できないものがある場合、チーム内で連携をとりながら認識齟齬のないよう確認することで、自身のスキル向上にも繋がるためどんどん質問・確認をしていきましょう。

コード品質系

1. Null安全

型定義の際にAPIから受け取る値や、データの持ち方などの都合でnullableにすることもあるかと思います。
ただし、値を扱う際にNull安全が保たれていないと実行エラーが発生したり、エラー表示はされないものの意図した動作にならない場合があります。

例えば

function variableString(target?: number | null) {
    return `number ${target}`
}

のような関数があった場合、nullやundefinedがそのまま文字列として表示されてしまうことになります。

対策

NullやUndefinedが許容されている場合、本処理の前にNull安全のための判定式を実装するようにしています。
判定後のハンドリングや、そもそものNullable型の使用ルールについては、チーム内で議論して最適な規約が作れるとよさそうです。

2. 不要な条件分岐・繰り返し処理

例として、特定の数字のみ別の処理を行いたい場合の処理として

const hoge: number = 1

function example(item: number) {
    if (hoge === 1) {
        console.log('one') 
    }　else if (hoge === 2 || hoge === 3) {
      console.log('alt')
    }
}

このような記述があるとします。
もちろんこのままでも動作するのですが、特定の数字のみ判定したいのに判定式が複数あってはパフォーマンスや可読性に問題が出てしまう可能性があります。

対策

ケースバイケースではありますが、条件分岐を極力減らすことを意識して早期リターンを行うようにしています。

const hoge: number = 1

function example(item: number) {
  if (item === 1) {
    console.log('one')
    return 
  }
  console.log('alt')
}

このような簡単な処理の場合は問題ありませんが、型判定や複数の条件が絡み合っている場合は見落としてしまいがちなため、個人的に気をつけています。

まとめ

プロジェクトやメンバーのスキルセット等でコーディング規約や習慣は異なるものかと思います。
ここで挙げた例はあくまで一例でしかありませんので、レビュワー・レビュイーの双方が納得できるようなPRレビューを心がけたいですね。

ある程度分類ごとにまとめてみましたが、まだまだ細かい点では記述しきれていない(特に品質系)ため、機会があれば随時更新して来たいと思います！