Permalink
6 comments
on commit
sign in to comment.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Observers are no longer important enough to get this configuration op…
…tion called out at the top level
- Loading branch information
Showing
1 changed file
with
0 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could someone elaborate on why observer are no longer important enough?
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consider observers to be a minority use case at this point. They're like aspects, you don't know what they're watching and why when you look at existing code. It's generally better to have explicit calls in filters or directly in controllers and models.
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, I used to use Observers to trigger when emails would be sent on signup and we had the idea of the Sweeper for caches. Now I just let the controllers send out emails (please don't use after_create for that!) and key-based expiration has eliminated the need for manual cache expiration.
In fact, if someone showed me observers today, I'd probably say "that'd make a nice plugin, but it's not core material".
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your example, as far as I understand it, I would agree that moving from the Observer to the controller is a clearer approach.
But lets say I want to implement a Notifications System,
which creates notifications for the creation of a couple of models (posts, comments, etc).
I much prefer the idea of an Observer which observes these models instead of duplicating the same callback in each of them.
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I see dead trolls coming)
@dhh do you intend to also remove the observer layer from core, in the future, or will it stay as is, just not mentionned in generated config files ?
775ddf2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oelmekki The current plan is to split observers into a plugin. Should be done soon in master.
@wpp I don't think an Observer is a good solution in that case either. Use a controller concern module that encapsulates the pattern on that side.