【プルリク出す前に確認!】コードを書くとき注意すること
こんにちは、技術部のNです。 今回は、正解があるようでないようである(?)コードの書き方において、自身も注意しているとても基本的な内容について書きたいと思います。 言語はRailsで書いていますが、内容はRailsに限ったことではないと思いますので、皆さんの使われている言語に置き換えて考えていただけるといいと思います。
クエリ数が多くなっていないか
n + 1問題もそうですが、クエリ数が無駄に多くなってしまうコードだと、その分DBへのアクセス回数が増えパフォーマンスが悪くなってしまいます。
例えばReservation(予約)モデルのいくつかのインスタンスのIDが変数reservation_idsに配列で入っており、そのIDの予約が有効enabledかを確認し、1件でも有効でないものがあった場合はfalseとしたい時、
```
reservation_ids.all? { Reservation.find(_1).enabled? }
```
このようなコードでは、IDの数だけfindで毎回取得してクエリ数が多くなります。
```
Reservation.where(id: reservation_ids, enabled: true).size == reservation_ids.size
```
一例ですが、このようにすると、取得は1回でIDが全て有効な予約のものか確認することができます。
条件分岐やメソッド名の分かりやすさ
他の人がそのコードを読んだときに、何をしているコードなのか、メソッドであればどんな処理を担っているのかが、その名前で分かりやすいと良いと思います。
条件分岐ならば、
```
unless reservation.exists?
```
よりも
```
if reservation.empty?
```
の方が瞬時に理解しやすいですね。
メソッド名ならば、 ・メソッドが何をするのかが分かること ・どこまでを担っているかが分かること ・返り値の状態 等が考慮されているかを確認します。
ある予約が、更新可能なステータスかを確認するメソッドがあったとします。
```
def updateable_reservation?
# 更新可能なステータスかを確認する処理
end
```
しかし予約が更新可能かどうかは、ステータスと予約日時の判定が必要だった場合、updateable_reservation? という命名では、更新可能な予約かどうかの全てのチェックを行ってくれそうな気がします。ステータスの判定しかしていないのであれば、updateable_status?の方が適切な気がしますし、命名をそのままにする場合は、処理の中に予約日時の判定も含めると、メソッド名で何の処理をどこまで担っているかが分かりやすいです。
また返り値がtrue/falseになる場合はメソッド名をxxx?としたり、処理内でエラーが発生するコードがある場合は、メソッド名をxxx!としたりすると、よりメソッドの内容が分かりやすくなります。
以上、基本的なことですが、コードを書くときに注意していることのご紹介でした。少しでも参考になれば幸いです。最後までお読みいただきありがとうございました。
関連記事
- 2023-09-01
- テクノロジー