Ben and Joe discuss the dos and (mostly) don'ts when it comes to ActiveRecord callbacks. You'll find out why you're generally better off not using this feature, with several suggestions for better alternatives.
This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/callbacks
What about observers? I havenât personally used them, but would put them in the same category as callbacks?
Seems like they hook into the object the same way callbacks do, so would be subject to the same failings/dangers?
Also, what do you all think about a pub/sub type architecture, something like the wisper gem?
I havenât personally used either of these methods, and have gone the route of something similar to what Ben & Joe did in the video. However, I have come across these other strategies and would love to get other peopleâs opinions on them.
@jferris @benorenstein I agree in general on callbacks being evil(most of the times) but consider this example:
We have a model NewsSource which represents a blog with corresponding info like author email, site_url, title, etc.
Every news source also has a rss feed. The feed could be atom, rss, or whatever type.
There is a parser in the app that fetches feeds periodically and parses the posts.
Initially we kept feed_url value in news_source itself but after constructing parsers certain way we decided to extract feed data out to have some validations and other checks.
So at the end we have two very tightly coupled models:
class NewsSource
has_one :feed
end
class Feed
belongs_to :news_source
end
Every time we create or update a news_source we need create or update corresponding nested/child object feed and this is where before_create :generate_feed and before_update :update_feed callbacks came in handy.
Ideally we shouldâve used accepts_nested_attributes_for but in our case the client apps(namely our Ember.js admin panel) doesnât send nested objects to our API endpoints so we had to wrap it this way.
What do you think, is this one of those âexceptionâ cases when itâs ok to use callbacks or we can somehow improve it?
Thanks.
What perfect timing for this episode. I was faced with a situation where I had to shutdown an application another dev programmed that I did not understand on Wednesday. Long story short was that it is an app that added numbers into our inventory system via an API. It ran into a situation where a specific attribute was changed and there was a before_save callback. Details omitted for brevity, the callback created a transaction issue, but the API call was already made. 24+ hours later before it was noticedâŚ, blah. I wish it was as simple as reversing the inventory, but as of today, the numbers are still not correct after several attempts.
I rewrote that part of the application and extracted a new model and it is much more explicit as to what is going on now.
This is actually one of those situations I find particularly confusing with callbacks. I can never remember which model will be saved when, or what ActiveRecord does with unsaved associations, validations, and foreign keys. I prefer to tell ActiveRecord to save single models, and handle larger transactions in their own object.
If youâre doing this from a form, making an object which encapsulates the form and all the objects it updates is nice. You can delegate and combine associations, and then clearly and concisely save all the objects in one transaction.
If youâre doing it in a backend service, you can do the same thing, except you donât really have to worry about validations or messages. Any failures can be handled by exceptions (usually raised by save!).
ActiveRecordâs observers have never really received first-class treatment in Rails, and they were removed from Rails itself in 4.0.
I havenât used Wisper, but Iâve had good luck with similar architectures. There are some common things which make observer/pubsub/events different than callbacks. The ActiveRecord callback API is a way to hook into the lifecycle of an object, modifying its behavior. For example, you can add new validation errors and perform other side effects which actually change the return value of a save call. Events are best when used exclusively for side effects. For example, triggering emails or updating remote service after a save is successful is a good use for an observer or event.
In a way, using observers or something like Wisper is going to the extreme, OOP end of the âTell, Donât Askâ philosophy: everything is phrased as a command, and dependencies are managed through events rather than composition. Besides dependency injection, events are the only way I know of to implement Inversion of Control.
1 Like
I tried to avoid callbacks, then I create a service class to handle logics after model created, but I still feel not happy about it.
# comments_controller.rb
def create
post = find_post
comment = post.comments.new(protected_params)
if comment.valid?
CreateComment.new(current_user, comment).create
render_created(comment)
else
render_bad_request(comment)
end
end
# create_comment.rb
class CreateComment
def initialize(user, comment)
@user = user
@comment = comment
end
def create
save_comment
subscribe_to_post
update_rank_score
create_activity
end
private
def save_comment
@comment.save!
end
def subscribe_to_post
@comment.post.add_subscriber(@user)
end
def create_activity
CreateActivity.create(user: @user, action: 'new_comment', trackable: @comment)
end
def update_rank_score
UpdateRankScore.update(@comment.post)
end
end
Iâm not sure, but I feel CreateComment its job doing too much, and it makes hard to tests each functionality, and my test feel very tight couple to implementation:
require 'spec_helper'
describe CreateComment, '#create' do
let(:user) { double(:user) }
let(:post) { double(:post, add_subscriber: true) }
let(:comment) { double(:comment, post: post, post_id: 1, save!: true) }
before do
UpdateRankScore.stub(:update)
CreateActivity.stub(:create)
end
it 'creates comment' do
create_comment(user, comment)
expect(comment).to have_received(:save!)
end
it 'subscribes to the post' do
create_comment(user, comment)
expect(post).to have_received(:add_subscriber).with(user)
end
it 'updates post rank score' do
create_comment(user, comment)
expect(UpdateRankScore).to have_received(:update).with(comment.post)
end
it 'tracks activity' do
create_comment(user, comment)
expect(CreateActivity).to have_received(:create).with(
user: user,
action: 'new_comment',
trackable: comment
)
end
def create_comment(user, comment)
CreateComment.new(user, comment).create
end
end
@samnang I think this is a case where multiple event listeners or subscribers would be a good fit. Youâd fire an event/publish a topic if the save is successful. Then your spec only needs to test that the event gets fired, or you could write a slightly higher level spec that tests the behaviour from the integration between CreateComment and its related listeners
In this episode, a decorator is used to avoid a callback:
class Comment < ActiveRecord::Base
end
class FacebookNotifyingComment < SimpleDelegator
def initialize(comment)
super(comment)
end
def save
if __getobj__.save
post_to_wall
end
end
private
def post_to_wall
Facebook.post(title: title, user: author)
end
end
class CommentsController < ApplicationController
def create
@comment = FacebookNotifyingComment.net(Comment.new(params[:comment]))
if @comment.save
redirect_to blog_path, notice: "Your comment was posted."
else
render "new"
end
end
end
Why is the decorator advantageous over inlining the Facebook notification within the controller?
class Comment < ActiveRecord::Base
end
class CommentsController < ApplicationController
def create
@comment = Comment.new(params[:comment])
if @comment.save
Facebook.post(title: @comment.title, user: @comment.author)
redirect_to blog_path, notice: "Your comment was posted."
else
render "new"
end
end
end
1 Like
I think that FacebookNotifyingComment is extracted in the name of Single Responsibility Principle. With that principle in mind you create thin one-task-focused classes which are easily testable, reusable, simple etcâŚ
Thin focused controller does two things (not including authentication stuff):
- Translates HTTP request to your applicationâs language, sending a message to one of your appâs classes.
- Translates response from that message to HTML or whatever format youâre using.
On the other hand you get 15 lines in your FacebookNotifyingComment instead of single simple Facebook.post in your controller. In the latter case controller is still pretty thin, you can quickly figure out what is going on and test it with Facebook stabbed out. But now controller is a part of your app, not a simple translator.
At the end of the day itâs all about design, balance and consistency.
1 Like
@lompy Got it, good points. I appreciate the decorator more with that knowledge.
Iâve seen this style of decorator (âdecoratorâ?) implementation recently in a real-world app:
class Comment < ActiveRecord::Base
end
class FacebookNotifyingComment < SimpleDelegator
def initialize(params)
super(Comment.new(params))
end
def save
if __getobj__.save
post_to_wall
end
end
private
def post_to_wall
Facebook.post(title: title, user: author)
end
end
class CommentsController < ApplicationController
def create
@comment = FacebookNotifyingComment.new(params[:comment])
if @comment.save
redirect_to blog_path, notice: "Your comment was posted."
else
render "new"
end
end
end
What do people think of that versus wrapping the comment object like a classic decorator?
1 Like
The Draper gem allows you do something similar. If you include the decorates_finders macro, you can do something like @article = ArticleDecorator.find(params[:id]).
I think maybe creating a builder/factory might be a better option than adding Comment within the FacebookNotifyingComment initializer. It would keep the comment Role dependency abstract rather than concrete within FacebookNotifyingComment
Very awesome screencast about callbacks!
One question though, will it be much better to always approach every callback problem using
a decorator or is there a scenario wherein a decorator/delegator is not applicable?
P.S Very cool vim theme. May I know what it is?
I have a Rails app with a model AssessmentForm which has many AssessmentItems. Whenever an AssessmentItem is updated I have to touch its AssessmentForm model. I have implemented it using callbacks. @jferris would you consider this as an acceptable case for using callbacks?
As per your discussion you pointed out that callbacks make it difficult to isolate two actions.In my use case if we consider the domain model it seems logical to me that every time an AssessmentItem changes the AssessmentForm has changed so I donât foresee a need to isolate the two actions(updating an AssessmentItem and changing the updated_at attribute of the AssessmentForm). What pitfalls do you see in using callbacks in this case?