Rails after_initialize only on "new"

Question

I have the following 2 models

class Sport < ActiveRecord::Base
  has_many :charts, order: "sortWeight ASC"
  has_one :product, :as => :productable
  accepts_nested_attributes_for :product, :allow_destroy => true
end

class Product < ActiveRecord::Base
  belongs_to :category
  belongs_to :productable, :polymorphic => true
end

A sport can't exist without the product, so in my sports_controller.rb I had:

def new
  @sport = Sport.new
  @sport.product = Product.new
...
end

I tried to move the creation of the product to the sport model, using after_initialize:

after_initialize :create_product

def create_product
 self.product = Product.new
end

I quickly learned that after_initialize is called whenever a model is instantiated (i.e., from a find call). So that wasn't the behavior I was looking for.

Whats the way I should be modeling the requirement that all sport have a product?

Thanks

Abrasion

Answer 1

Putting the logic in the controller could be the best answer as you stated, but you could get the after_initialize to work by doing the following:

after_initialize :add_product

def add_product
  self.product ||= Product.new
end

That way, it only sets product if no product exists. It may not be worth the overhead and/or be less clear than having the logic in the controller.

Edit: Per Ryan's answer, performance-wise the following would likely be better:

after_initialize :add_product

def add_product
  self.product ||= Product.new if self.new_record?
end

Corrinnecorrival

The problem I have using this solution is that in my case, the product field can be null (so I really need an after_initialize only on create).. if anyone has an idea, would be great, thanks! – Megilp

@Megilp Check out before/after_create. If your logic is too complicated, you probably don't want to hide it in a before/after hook as that could be too magical. – Corrinnecorrival

This method however is not as optimized, once you start selecting large amount of sports/products, you will see that your query is very unoptimized because for every relationship you have you are doing a select statement to see if the product exist – Pneumatics

@Pneumatics You should probably use includes on your query - see the Eager Loading section at api.rubyonrails.org/classes/ActiveRecord/Associations/… – Corrinnecorrival

Putting logic on controller is NEVER a good option... using rails callbacks keeps you creating new Products every time some finder retrieves something from DB – Humeral

@RudySeidinger A major point of MVC and good separation is clarity. Using Rails callbacks hides functionality. Sometimes that's good, sometimes that's bad. It's certainly not a case where you can use words like "never" or "always". And the code as shown only creates a new product if no product exists (this was the point of the OP question). – Corrinnecorrival

@bostonou, i disagree. By putting any business logic on your controller, you're tiding your business rules with your application layer, you're violating single responsability principle and, definitely, that's never a good ideia...Notice that i'm talking about putting logic on the controller. The callback problem is different. – Humeral

@RudySeidinger I almost agree with you but is it really a business rule, after all? If it were a business rule, we would use after_create, but we can't because... the controller needs an unsaved object. Can't it be the job of the controller to fulfill its own specific needs? Or, at least, shouldn't this code be put in a specific method in Sport (Sport.new_sport_with_product), rather than making a binding rule that all records should fulfill. Just wondering. – Oria

@Eric L, When i first read the problem, i thought that creating a Sport object with a Product is a pre-condition that exists for the model and the business rules tied to it. If it really is, IMO this logic must exist on the domain layer not the view/app layer (which include the controllers). The class method you've suggested is a good option if not all sports must have at least a product – Humeral

Answer 2

Surely after_initialize :add_product, if: :new_record? is the cleanest way here.

Keep the conditional out of the add_product function

Seleucia

does after_initialize :add_product, on: :create work? – Ingrowing

Looks like not: I get: ArgumentError: Unknown key: :on. Valid keys are: :if, :unless, :prepend – Elope

Answer 3

If you do self.product ||= Product.new it will still search for a product every time you do a find because it needs to check to see if it is nil or not. As a result it will not do any eager loading. In order to do this only when a new record is created you could simply check if it is a new record before setting the product.

after_initialize :add_product

def add_product
  self.product ||= Product.new if self.new_record?
end

I did some basic benchmarking and checking if self.new_record? doesn't seem to affect performance in any noticeable way.

Bandage

Great work with the performance hit by invoking new_record? ! – Pricecutting

It is also possible to move the new_record? check out of add_product by writing after_initialize :add_product, :if => :new_record?. In some cases that will be better-organized. – Banquer

How would one have known they could put :if there? Can you link to documentation instead of being the documentation? – Acacia

Answer 4

Instead of using after_initialize, how about after_create?

after_create :create_product

def create_product
  self.product = Product.new
  save
end

Does that look like it would solve your issue?

Greensboro

This is for the new method in the controller....nothing gets saved to the db yet, so after_create won't be called. – Abrasion

Answer 5

after_initialize :add_product, unless: :persisted?

Tremolant

Remember that Stack Overflow isn't just intended to solve the immediate problem, but also to help future readers find solutions to similar problems, which requires understanding the underlying code. This is especially important for members of our community who are beginners, and not familiar with the syntax. Given that, can you edit your answer to include an explanation of what you're doing and why you believe it is the best approach? That's especially important here, when there are established answers to this question. – Fungicide

Answer 6

It looks like you are very close. You should be able to do away with the after_initialize call altogether, but first I believe if your Sport model has a "has_one" relationship with :product as you've indicated, then your Product model should also "belong_to" sport. Add this to your Product model

belongs_to: :sport

Next step, you should now be able to instantiate a Sport model like so

@sport = @product.sport.create( ... )

This is based off the information from Association Basics from Ruby on Rails Guides, which you could have a read through if I am not exactly correct

Shien

The polymorphic association throws this off a bit. A product belongs to a productable (made up word). This is because some products are sports and some are movies (I'm starting with sports). This is my understanding of how to handle inheritance in Rails (other than Single Table Inheritance, which I didn't want). Thanks for the idea though! – Abrasion

Ah I see, I hadn't realized the polymorphic nature of the model, and in turn have realized I don't know how to handle this either. I will continue to look in to it on my own as well. I'll let you know if I come up with anything. – Shien

This railscast seems to contain the answer you are looking for. Looks like you use a similar way of creating the new sport as I attempted above. Hope this helps. – Shien

Yep, looks like the answer is to put the logic into the controller. That's what some ppl said over at the IRC channel too – Abrasion

Answer 7

You should just override initialize method like

class Sport < ActiveRecord::Base

  # ...

  def initialize(attributes = {})
    super
    self.build_product
    self.attributes = attributes
  end

  # ...

end

Initialize method is never called when record is loaded from database. Notice that in the code above attributes are assigned after product is build. In such setting attribute assignment can affect created product instance.

Damson

You should not do this for a number of reasons detailed here: #4377492 – Omnibus

No, there is no reason not to override initialize. Contrary, if you look into rails source code, you'll understand that initialize was meant to be overridden. All you need to do is to call super in your overridden initialize, but it's a rule that is expected to be followed by all ruby developers and rails developers silently expects you to do it. – Damson

I'd be wary of doing this as it's the kind of thing that can cause confusion, break in future versions, and possibly cause problems for certain third-party gems. unless it's very officially advised. I'm all for general hacks/workarounds, but the constructor is a particularly fragile feature and after_initialize offers a straightforward alternative. – Nickynico

I agree with @VictorNazarov, an after_initialize callback is generally a bad idea as it has two drawbacks - it creates an overhead for all future initializations, and breaks custom SELECTs, whereas an overridden initializer has neither of those. – Tittle

Answer 8

before_create is perfect for this particular case.

You see, if you want to initiate some block of code on nearly created record you should use before_create hook because it would be called only once - when record has been created. No on update neither on save. Only once - when record is created.

my_model = Model.create(**record_params) # this will trigger before_create hook
my_model.update(**some_params) # this will not trigger before_create hook 

Nitre