過剰なルーティングのカスタマイズ
元記事はこちら
翻訳
Bad Smell
map.resources :posts, :member => { :comments => :get,
:create_comment => :post,
:update_comment => :put,
:delete_comment => :delete }
Roy Fielding’*1の論文*2によると、リソースと、その状態を表すには、RESTfulなルーティングを使うべきです。過剰なカスタマイズをせず、デフォルトの9アクション(index, show, new, edit, create, update and destroy)を使いましょう。過剰なカスタマイズを避けるには、別リソースを使うようにしましょう。
Refactor
map.resources :posts do |post| post.resources :comments end
postリソースに対して、create_comment, update_comment and delete_commentというカスタムルーティングを使わないようにするために、commentsリソースのnewアクションを使うようにします。できる限りデフォルトの7アクションを使うようにしましょう。
感想
RESTfulを意識しましょう。デフォルトのアクション以外を作りたくなったらコントローラーを分割しましょう。ということですね。
RESTfulにするならデフォルトのルーティングを使わない
元記事はこちら
感想
内容がRails2の時代のものでRails5と書き方が大きく異なるので、翻訳は割愛。 RESTfulなルーティングを採用する場合は、余計なルーティングが生成されると、セキュリティリスクにもなるからやめましょうという主旨だと思う。
resources :posts, :member => { :push => :post }
みたいな書き方をするのは７種類のアクション(index、show、new、edit、create、update、destroy)全部を使う時だけにして、 一つでも欠ける時は個別に書くのがよい。と思う。
コーディング規約が浸透しない問題
コードレビュー、プルリクのタイミングなどでは、レビュアーの指摘は立ち位置をそのプルリク作者の思考地点に揃えるのが私は良いと思う。その前提が作れなさそうなら、実装方針を事前に揉むってことが必要だと思う。そういう流れが作れると割とスムーズになる。
— Hidenori Goto (@hidenorigoto) 2017年5月14日
2017/5/15 20:08 追記
システム開発で、レビューする側もレビューされる側も、成果物をレビューするという意識になっていると不幸になる。作業途中で方向性が間違っていないか？をレビューする
— てるろー (@terurou) 2017年5月14日
コーディング規約をつくっても読まれなかったり、定着しなかったりで、うまくいった試しがなくて、 なんでかなーと思っていたけれど、このツイートをみて、「実装方針の共有」ってところがキモになるんじゃないかと思った。
今まで見てきた/作ってきたコーディング規約だと、コーディングスタイルの話と、実装方針の話がごっちゃになって箇条書きにされているだけというのが多かった気がする。 コーディングスタイルを統一させたいのであれば、PHPならphpcsとか、rubyならrubcopとかあるし、Scrutinizerとか、SideCIでも静的解析はしてくれるので、そういうものを人間が目で確認しなくてもいいように取り入れていくべき。
実装方針については、フレームワークやライブラリによってある程度縛られる部分はあるし、エンジニアなんだから使っている技術は十分に知っとけっていう人もいるかもしれないけれど、スポットで入る人とか、初めて触る人との共有はやはり難しいと思うので、「こういう処理が書きたくなったら、このコンポーネントにまとめてね。」とか「この部分はテストパターンも厚めに書いてね」とか、共有できると、PR見る方も、投げる方も幸せになれるような気がする。
OSSでなくてもCONTRIBUTING.md作ってあらかじめ共有しときたいことは書いておいたほうが良いのだろうか。それでもやっぱり読まない人は読まないから、無駄なんだろうか。 「これくらい知っとけ」の場合でも、「この記事は読むべし」みたいな一覧があると、ミスコミュニケーションが原因でPR全部書き直しみたいな事態は防げそうだけど、大変でも苦労して頑張れ事案なのだろうか。
入れ子モデルフォーム
元記事はこちら
翻訳
Bad Smell
class Product < ActiveRecord::Base
has_one :detail
end
class Detail < ActiveRecord::Base
belongs_to :product
end
<% form_for :product do |f| %>
<%= f.text_field :title %>
<% fields_for :detail do |detail| %>
<%= detail.text_field :manufacturer %>
<% end %>
<% end %>
class ProductsController < ApplicationController
def create
@product = Product.new(params[:product])
@detail = Detail.new(params[:detail])
Product.transaction do
@product.save!
@detail.product = @product
@detail.save
end
end
end
ProductモデルとDetailモデルには１対１の関係があり、productと一緒にmanufacturerも保存したいとします。この場合、productオブジェクトとdetailオブジェクトを生成し、トランザクションによって関連付けます。でも、なんでこんな込み入った処理をコントローラーでやっているのでしょう。Productモデルがdetailの生成を担当すべきです。
Refactor
class Product < ActiveRecord::Base
has_one :detail
accepts_nested_attributes_for :detail
end
<% form_for :product do |f| %>
<%= f.text_field :title %>
<% f.fields_for :detail do |detail| %>
<%= detail.text_field :manufacturer %>
<% end %>
<% end %>
class ProductsController < ApplicationController
def create
@product = Product.new(params[:product])
@product.save
end
end
Productモデルに、accepts_nested_attributes_forを追加したので、ProductモデルはDetailモデルの生成を制御できるようになりました。これは良いことです！
accepts_nested_attributes_forのおかげで、１対多の関係における生成も簡単にできるようになります。
class Project < ActiveRecord::Base
has_many :tasks
accepts_nested_attributes_for :tasks
end
class Task < ActiveRecord::Base
belongs_to :project
end
<% form_for @project do |f| %>
<%= f.text_field :name %>
<% f.fields_for :tasks do |tasks_form| %>
<%= tasks_form.text_field :name %>
<% end %>
<% end %>
railsに素晴らしいメソッドが用意されていて、実にありがたいことです!
感想
Railsガイドの関連付けに関する章を最近読みながら書いてますが、いまいち理解できていない。 関連付けの仕方はわかってきたものの、生成のさせ方と検索時に取れるレコードの形（Assosiation/Array）の判別がいまいち腑に落ちていないというか、モヤっとしている。 今日もちょうどBadSmellなコードを書いてしまったので、テスト書いてリファクタリングせねば。。
