【読書記録】コードレビューで学ぶRuby on Rails
コードレビューで学ぶ Ruby on Railsを読んだので、
勉強になったことと読んで感じたことをまとめます。
なお、怠慢により買ってから１ヶ月以上経過しています。
前置き
自分の状況
- 未経験からエンジニアになって１年５ヶ月
- Ruby on Railsは会社に入ってから覚えた
- 最近コードレビューする機会が増えてきた
- 色んな記事や本を読んでRails的な書き方や設計などについて勉強中
勉強になった箇所
1.6 データベースに制約を設定する
TIPS:文字列カラムにNOT NULL と合わせて空文字をデフォルト値をつける が勉強になりました。
他の人が作成した文字列のカラムにNULLが入ることがあり、
Rails側での対応方法に悩んでいたケースがありました。
たったこれだけの制約ですが、これだけで実装の悩みが解決するので、
自分がDB設計する際にこの制約をつけるようにしたいと思います。
2.1 ビューに複雑なロジックを書かない
「Fat Controller は回避するべき」とはよく聞きますが、それ以前にFat Viewはもっと回避されるべきです。
気を抜くとViewに条件分岐が発生するので、Decoratorを作成する対応をコードレビューで依頼することが多いです。
View Componentは使いたいですが、関わっているレポジトリではGemがインストールされておらず、導入事例やルールを0から決めるほどの労力を割くほどではないと考えています。
helperはどこでも呼び出せて、無秩序になりがちなのであまり使用しないようにしています。
（コントローラーで呼び出すのは勘弁してほしい…）
2.4 時間、日付のフォーマットにはI18n.lを使おう
これについては初めて知りました。積極的に使いたいと思います。
nilの場合も含めて勉強になりました。
3.1 一覧を取得する際にはorderをつける
これも意識していなかったので、勉強になりました。
少し内容から逸れますが、created_atでorderしていた箇所があり、
呼び出す場所では日付しか参照していなかったので、
同じ日に２レコード作成された場合に、バグが発生するケースがありました。
今後は順序には気をつけたいです。
3.2 安全にオブジェクトを取得する
これも意識する機会が少なかったので、勉強になりました。
関連先のidでレコードを取得するケースだと37ページのようなコードをこれまで書いていました。
提案の部分の例のようにログインユーザーを起点として値を取得することを始めとして、
関連付けがある場合は関連元のオブジェクトから取得するように意識することにします。
4.2 テストコード単体で期待値の根拠を示す
基本的にFactrybotに書けば事足りる内容はFactorybotに全て寄せていましたが、
この節を読んで、認識を改めました。
テストがドキュメントの代わりをするという考え方を踏まえても、
期待値に使う値をテストコードの内で準備しておいた方が、
テストコード単体で内容が完結してメンテナンス性が上がると感じました。
4.5 入力値や期待値はベタ書きにする
これも意識していないと変数を使ってしまうことが多いので、勉強になりました。
4.6 factory_botのtraitを使って書く負担を減らす
これは自分も心がけており、コードレビューで指摘することも多いです。
今回勉強になったのは、デフォルトで関連レコードは作成しないようにすることです。
あるレポジトリからテストコードを何も考えずにコピーしてきたところ、
レポジトリ間でFacotrybotの関連付けの設定が異なっていたため、
テストコードを書き直すことになりました。
traitを使っていれば、このような事態は避けられたと思います。
コードレビューで気になったこと
本論とはあまり関係ないですが、
Railsのコードレビューをしていて気になったことを書きます。
Modelの内容をViewに直書きする否か
例えば次のようにArticleモデルにcategoryカラムがあり、
特定のカテゴリのみを抽出するscopeを作成したとします。
class Article < ApplicationRecord
SPECIFIC_CATEGORIES = ['Category1', 'Category2', 'Category3', 'Category4', 'Category5', 'Category6', 'Category7', 'Category8', 'Category9', 'Category10', 'Category11', 'Category12']
scope :specific_categories, -> { where(category: SPECIFIC_CATEGORIES) }
end
これをViewで呼び出す際、しばらくは次のように書いていました。
class ArticlesController < ApplicationController
def アクション名
@specific_articles = Article.specific_categories
end
end
<%= form_with(model: @article, url: articles_path, method: :get) do |form| %>
<%= form.select :category, options_for_select(@specific_articles), prompt: 'カテゴリを選択してください' %>
<%= form.submit '検索' %>
<% end %>
しかし、今年の春に次の記事を読んでから下の記述に変更しました。
【新人プログラマ応援】Railsにおける良いコントローラ、悪いコントローラについて
<%= form_with(model: @article, url: articles_path, method: :get) do |form| %>
<%= form.select :category, options_for_select(Article::SPECIFIC_CATEGORIES), prompt: 'カテゴリを選択してください' %>
<%= form.submit '検索' %>
<% end %>
ということで、コードレビューする際にも
このような記述を見かけたら指摘しているのですが、
「いや、モデルの内容をViewに直書きするのは良くないからコントローラーに書く」というコメントをされました。
上の記事にも
個人的には「どうしても」というとき以外はやらない方がいいと考えています。
(太字は自分が追加)
とあるのでどこまでをModelとViewとControllerどこに書くべきか
人によって考え方に差はあるようです。
Discussion