(rails5)モデルクラスにinitializeメソッドを付けると何か不具合が起こり得る可能性はあるのでしょうか？

Question

例えば以下の様にモデルとコントローラーがあるとします。
モデルクラスにはコンストラクタ(initialize method)を付け加えており、インスタンス化したさいに自動的にidをセットするようにします。

class Hoge < ApplicationRecord
　
  def initialize(id)
　　＠id ＝ id
  end

  def hoge
    Hoge.where(id: @id)
  end

end

class HogesController < ApplicationController

  def create
   fuga = Hoge.new(current_user.id)
   @foo = fuga.hoge
   
   Hoge.new(id: current_user.id, another: id, delflag: 1)
   Hoge.save!
  end


end

ここで質問なのですが
hogeというHogeクラスに記載されているインスタンスメソッドを使いたい際は、

fuga = Hoge.new(current_user.id)
foo = fuga.hoge

のような形でインスタンスメソッドを呼び出せると思います。
同じく、モデルを保存したい時はnewメソッドを用いてインスタンスオブジェクトを生成すると思いますが、
保存したいモデルの属性をnewメソッドの引数に取るに辺り、自前で実装したコンストラクタの引数と数が異なっていると思います。

Hoge.new(current_user.id) #1つ
Hoge.new(id: current_user.id, another: id, delflag: 1) #3つ

このせいかは良く分かっていないのですが、new,save!メソッドを使用すると以下のようなエラーが出ている状態です。

undefined method `[]=' for nil:NilClass (NoMethodError) 

よくよく思うとネットのリファレンスを見るにあたり、initializeメソッドをモデルクラスに記述しているものを見たことが無く
自分の実装方法が合っているのか気になります。
質問をまとめると
①以上の様に異なる用途でnewメソッドを用いたい際に引数が異なっていても問題がないか
②createする際のインスタンスオブジェクト生成のnewとインスタンスメソッドを使用したい際のnewは何が違うかということです(原則同じことをやっているのでしょうけど、なにか継承元のApplicationRecordクラスが悪さをしている気がします。。。)

長くなってしまいましたがよろしくお願いいたします。

---

追記

MVCにおいてはビジネスロジックはモデルに書いてモデルを太らせるのが鉄則といいますが、
以下のようにのようにモデルクラスに複数メソッドを書き、where句等で同じ変数を用いたい際にコンストラクタがあるとインスタンスメソッドを呼び出す際に書き方がスッキリして便利だと思うのですが、モデルクラスでコンストラクタを使いたい際はどのようにするのがベストなのでしょうか。

#コンストラクタあり
class Hoge < ApplicationRecord
　
  def initialize(id)
　　＠id ＝ id
  end

  def hoge
    Hoge.where(id: @id)
  end

　def hoges
    Hoge.includes(:bar).where(id: @id)
　end

end

#コンストラクタなし

class Hoge < ApplicationRecord

  def hoge(id)
    Hoge.where(id: id)
  end

　def hoges(id)
    Hoge.includes(:bar).where(id: id)
　end

end

それともそもそもrailsのモデルクラスの場合はPOCOではないのでロジックをあまり書くべきではないのでしょうか？

Answer 1

(rails5)モデルクラスにinitializeメソッドを付けると何か不具合が起こり得る可能性はあるのでしょうか？

この実装では、むしろ不具合しか起きないといっても過言ではありません。

よくよく思うとネットのリファレンスを見るにあたり、initializeメソッドをモデルクラスに記述しているものを見たことが無く

自分の実装方法が合っているのか気になります。

間違っています。このように、コンストラクタを上書きしてしまうと、

• 本来の引数で呼べない
• ActiveRecord::Baseのコンストラクタが呼ばれず、正常に初期化されない

などの問題だらけとなってしまいます。

Comments

[maisumakun]

えっと、HogesController#createでfugaはメソッド呼び出し以外に特に使っていませんし、Hogeの拡張は行わずに「@foo = Hoge.where(id: current_user.id)」と書いてまずい理由は何なのでしょうか。

[widget11]

ご回答ありがとうございます。
追記してみたのですが、単純にモデルクラスにビジネスロジックを書いていく上でメソッドがどんどん増えていくことを仮定すると、コンストラクタでよく使う値を初期化しといた方が書き方がスッキリすると考えたのですが、、、少し冗長になりますが以上のような書き方でもしょうがないといった感じでしょうか？
そもそもApplicationRecordを継承していないプレーンなserviceのような別クラスを作った方がよいという感じでしょうか？
よろしくお願いいたします。

[maisumakun]

> コンストラクタでよく使う値を初期化しといた方が書き方がスッキリすると考えたのですが

コンストラクトして、その値をほぼ使わない、ということのほうが意味不明です。

[maisumakun]

> モデルクラスでコンストラクタを使いたい際はどのようにするのがベストなのでしょうか。

モデルクラスのコンストラクタは作り付けですので、書き換えてはいけません。

…というより、上書きしたHoge#initializeで作りたいものは、明らかに「Hogeのレコードに対応するモデル」ではないですよね。

（検索条件を固定的に作り込みたい場合、scopeという専用の機能がありますのでそちらが適切です）

[widget11]

scope使用してみます。
最近やっとコンストラクタの使い方を覚えてきたため無理くり使おうとしていました。
ありがとうございます。