giftee engineer blog

Kaigi on Rails "Code Review Challenge" 正解発表 (Day1)

2023-10-28

Kaigi on Rails 2023 のブース企画 "Code Review Challenge" Day1 の正解発表です。

ブース写真

こんにちは、エンジニアの @megane42 です。

このたび、株式会社ギフティは Kaigi on Rails 2023 に Ruby Sponsors として参加させていただいております。イベント当日のスポンサーブースにて、“Code Review Challenge” というちょっとした Rails クイズのような企画を出展しています。この記事では、初日に出題した 3 問の正解発表を行っていきます。ブースにお越しいただいた方、本当にありがとうございます!

なお、別途 Kaigi on Rails 2023 の参加レポートを後日アップする予定なので、そちらもお楽しみに!

第 1 問 : RESTful ルーティング (難易度: ★☆☆)

正解は (2) です!

この設問は、いわゆる RESTful な URL 設計についての知識を問うものでした。今回は、とあるユーザーのギフト一覧を 取得 したいので HTTP メソッドは GET が適切でしょう。また、パスは “リソース名/リソースを特定するID” を反復する形式としたいので、今回は /users/123/gifts/ がふさわしいです。ということで、正解は (2) の GET /users/123/gifts/ でした。

Rails をレールに沿って使っていくと自然と RESTful な設計に近づいていくのも、Rails の魅力の 1 つですね。

第 2 問 : マイグレーションの罠 (難易度: ★★☆)

正解は (4) です!

この設問は、Rails の migration のリバーシブル性についての知識を問うものでした。選択肢 (4) のマイグレーションファイルは caution カラムの削除を意図したものですが、caution カラムの型についての情報が書かれていないので、rollback 時にどんな型のカラムを作ればいいかわからず、エラーになってしまいます。

「カラムの削除をしよう」ということに集中していると、テーブル名とカラム名さえあれば情報量的には十分だと思いこんでしまいがちです。マイグレーションファイルを書いたあとは試しに一度 rollback を実行してみるとよいかもしれません。

第 3 問 : 保守性の高いコーディング (難易度: ★★★)

こちらは記述式の回答でしたが、想定解は以下のような点でした。

  • (1) EmailChangingHistory モデルで ActiveRecord::Relation を返さない scope を定義している
  • (2) Gift モデルで default_scope を使っている
  • (3) Gift モデルの remind メソッドで、デメテルの法則に違反している

以下、各想定解についての簡単な解説です。

(1) について

Active Record の scope で ActiveRecord::Relation 以外のオブジェクトを返してしまうと、それ以降 scope チェーンを続けられなくなってしまいます。そのため、ActiveRecord::Relation または nil を返すように すべき と書かれています。

(2) について

default_scope については、Rails に備わった機能なので一概に悪とは言い切れないのですが、意図せずレコードが抽出されないことで思わぬバグをもたらす場合があるため、あまりおすすめできません。

(3) について

remind メソッドの中で email_address 変数を定義するときに user.email_changing_histories.latest.address といったコードを実行していますが、これはデメテルの法則に違反しています。user から現在設定中の email アドレスに辿り着くための経路がすべて明示的に書かれてしまっているので、将来その経路に変更が加わると動かないコードになってしまっています。できれば user.current_email_address のような呼び出しで取得できるようにするとよいでしょう。

まとめ

以上、Code Review Challenge Day1 の正解発表でした。のちほど Day2 の正解発表も載せる予定なので、お楽しみに!