読者です 読者をやめる 読者になる 読者になる

カッパでも分かるiOSアプリゲーム開発

カッパがひたすらゲーム制作に関することを書くブログです。Railsに関するTipsもたまにまとめてます。

【Rails】コードレビューでいただいた指摘メモ

Rails
スポンサードリンク

Ruby on Rails 5 超入門
f:id:InvokeTwoA:20151023183618p:plain「コードレビューが人を成長させる!」
f:id:InvokeTwoA:20151215172640p:plain「しかしコードレビューを導入したことで人間関係が崩壊したPJも沢山見てきました」
f:id:InvokeTwoA:20151023183618p:plain「減点評価しかないレビューは苦痛でしかないものね」
f:id:InvokeTwoA:20151215172640p:plain「相手を褒める! LGTM連打!  これが地味に大事だったりする」

【目次】

migration 変更で add_column したが、できれば created_at はテーブルの最後になるようにしたい。

add_column :users, :name, :string,  after: :email

みたいな感じで after を使うと良い

f:id:InvokeTwoA:20151023183618p:plain「確かに created_at と updated_at はテーブルの一番下にあると落ち着く」

before_filter、after_filter とかの書き方は古いよ

  • before_filter, after_filter と言った書き方は Rails3 の書き方
  • Rails4 以降では before_action, after_action という書き方が良い

f:id:InvokeTwoA:20151023183618p:plain「Railsのモダンな書き方って変わりすぎ」
f:id:InvokeTwoA:20151215172640p:plain「未だにRails3脳なのをなんとかせねば。時代はRails5なのに」

routing で collection, member は on 使えば一行で書けるよ

resources :user do 
  collection do
    get  :kappa
  end
end

resources :user do 
  get  :kappa, on: :collection
end

f:id:InvokeTwoA:20151023183618p:plain「好みの問題ではあるけど、ワンライナー歓喜」
f:id:InvokeTwoA:20151215172640p:plain「memberやcollectionの中身が複数行ならば前者を使おう、とのこと。個人的にはどっちでもイン! 山本くんでも〜佐藤くんでも〜イエ〜」

N+1問題のwarning出てるよ?

  • 下記のような感じで includes をつける
users.includes(:email_list)

asset ファイルへのアクセスは asset_path 使わないとダメだよ

  • precompile 時にファイル名にパラメーター付くので asset_path or asset_url で指定しないとエラーになる
  • 超あたりまえの事なのに、当時のカッパは見落としていた
  • ↑ローカルの開発だとプリコンパイルしないから気づかない。間抜け!
image_path =  "#{request.protocol}#{request.host_with_port}/assets/hoge.jpg"

image_path = asset_url("hoge.jpg")

リソース名に不可算名詞使うの、できるならやめよう

  • 不可算名詞に対応する処理入れてもいいけど、事故の元なのでそもそもリソース名を変えたいという指摘
  • ごもっともだぁ

f:id:InvokeTwoA:20151023183618p:plain「不可算名詞……」

  • ちなみに Sns というリソースだった。こいつは Sn という単数リソースと思われちゃうので色々なところで問題を生む困ったちゃん
  • 参考までにExternalService という名前に生まれ変わった

default_scope 使うのは事故の元だからやめよう

  • 全面的に同意
  • 途中からPJに入ってきた人とか絶対に混乱する事になる
  • scopeが効いてないぞ事件の真相は大抵が default_scope が犯人

json_spec 入れてるから json の一致はもっと簡単に書けるよ

result = JSON.parse(response.body)
expect(result).to have_key('hoge')

expect(response.body).to have_json_path("home")