はじめに
ruby 3.1.4
「新しいインスタンスを作成する際、既存のインスタンスの属性をコピーしたい」処理について。
Railsのコントローラのnewアクションでこんな処理をしていた。
ただDRYじゃないな～と思ったので、リファクタリング前と後でどういう風にしたか説明をさせてください。
リファクタリング前のコード
class HogeController < ApplicationController def new @hoge = Hoge.new return unless params[:id] copy_hoge(@hoge) end private def copy_hoge(new_hoge) original = Hoge.find(params[:id]) new_hoge.title = original.title new_hoge.content = original.content new_hoge.location = original.location new_hoge end end
なんやこれ？となってたけど、どうやら以下の流れのよう。
Viewで『コピーする』ボタンを押すと、コピー元のIDがparams[:id]として送信される。
これを利用して、「各属性をコピーしながら新しいインスタンスを作成する」という処理を実現しているみたい。
個人的に何がいやか？
- scaffold作成時に沿ったControllerになっていない
- コピーするべき属性が増えると、面倒くさそう
new_hoge.attribute = original.attributeという行をどんどん追加する必要が出てくる
- 同じ変数出てきすぎ。DRYじゃない。
ちなみに、scaffold作成時に沿ったControllerにすべき、っていうのは、以下の記事から。
リファクタリング後のコード
コントローラ
class HogeController < ApplicationController def new @hoge = if params[:id] Hoge.new_with_copied_attributes(Event.find(params[:id]) else Hoge.new end end end
モデル
class Hoge < ApplicationRecord class << self def new_with_copied_attributes(original_hoge) new_hoge = Hoge.new %i[title content location].each do |attribute| new_hoge.public_send("#{attribute}=", original_hoge.public_send(attribute)) end new_hoge end end end
何がよくなったのか？
- newアクション内を、Railsっぽい書き方に変えた。
- scaffold時に寄せたので、可読性が上がった(はず)
- コピーをするメソッドを、モデルのクラスメソッドとして、コントローラから分離した。
- テスト書きやすいし、FatControllerの予防になっている(はず)
- 各属性をコピーするメソッド内では
public_sendメソッドを使って、DRYにできた。
あくまで個人の見解。
public_sendメソッドとは
public_sendメソッドの前に、そもそもsendメソッドの理解から。
class Foo def foo() "foo" end end p Foo.new.send(:foo) # => "foo" p Foo.new.send("foo") # => "foo" # シンボル、文字列どちらでもOK
この方法で、オブジェクトの任意のメソッドを呼び出すことができる。でも、privateメソッドも呼べてしまう点が問題。
class Foo def foo() "foo" end private def private_foo() "private foo" end end p Foo.new.send(:foo) # => "foo" p Foo.new.send(:private_foo) # => "private foo"
こうすると、セキュリティガバガバ。できるだけ避けたい。そこでpublic_sendメソッドの出番。
名前の通り、publicなメソッドしか呼び出せない。
class Foo def foo() "foo" end private def private_foo() "private foo" end end p Foo.new.public_send(:foo) # => "foo" p Foo.new.public_send(:private_foo) # => public_send': private method `baz' called for #<Foo:0x00007f4e3ed1f978> (NoMethodError)
具体例を振り返る
リファクタリングでは、下記のような使い方をしていた。
%i[title content location].each do |attribute| new_hoge.public_send("#{attribute}=", original_hoge.public_send(attribute)) end
これは以下のように解読できる。
new_hoge.public_send("title=", original_hoge.title) new_hoge.public_send("content=", original_hoge.content) new_hoge.public_send("location=", original_hoge.location)
もっとシンプルに書き直すと、リファクタリング前と同じになる。
new_hoge.title = original.title new_hoge.content = original.content new_hoge.location = original.location
やっぱり
%i[title content location].each do |attribute| new_hoge.public_send("#{attribute}=", original_hoge.public_send(attribute)) end
が一番すっきりしている気がする。もし属性が増えても、%i[title content location]内をいじるだけでいいしね。
