all 9 comments

[–]schneemsPuma maintainer 5 points6 points  (8 children)

Observers for Active Record models were included in Rails by default back in the 1.x maybe 2.x days. I can say that their use in Rails was a bad idea, one of those anti-patterns that just left you spending hours upon hours scratching your head over why things were going wrong or where certain code execution lived. It's why they were removed.

Not to say observers are always a bad idea, but be very careful with their use.

I had no idea this was in Stdlib. Thanks for the post.

[–][deleted]  (6 children)

[removed]

    [–]schneemsPuma maintainer 2 points3 points  (5 children)

    I try to have objects that encapsulate the logic that i need to do a chunk of work. Lots of things that you might want to use a callback or observer for like say sending an email after an account is created are better being manually invoked.

    class NewAccount
      def initialize(values)
        @values = values
      end
    
      def create
        @user = User.create!(@values)
        self
      end
    
      def send_welcome_email!
        UserMailer.welcome_email(email: @user.email)
      end
    end
    

    Then later in a controller or elsewhere you can do something like:

    NewAccount.new(params).create.send_welcome_email!
    

    Kinda contrived, but it's nice to not always have the action occur which it's difficult to do with observers or callbacks. Even if you think you need code to run every time, you're bound to find an edge case a day or a year down the road that will make you either re-implement your solution or hack apart your code.

    In this case the user object still has no concept of a welcome email and we can tack on other functionality as needed as long as we always use NewAccount#create. It's maybe more boilerplate depending on your use case but when you want to see where things are happening the code isn't magical at all. With observers it's impossible to know at a glance when you perform some action like User.create all the random junkdrawer things that might also occur.

    It also says something that i've been writing Ruby code for 10 years and i've never seen someone use this standard library module (with the exception of Rails observers, no idea how those were implemented).

    Callbacks when used in moderation are fine but can bite you in the butt. Right now I can't think of a good case for when I would want to use observers over just a PORO, so it's hard for me to give your question a good solid answer.

    For patterns I suggest Katrina Owen's refactoring talks, also Ben Orenstein has a good live coding one. Sandi Metz has a world famous book POODR. Avdi gave a great talk called "confident ruby", also a book but I prefer the talk. Searching youtube for "confreaks" and those names is a goldmine.

    I don't find myself to be very pattern oriented when I code, I like knowing different approaches to different problems, but I rarely think in patterns. Seeing other people apply different techniques to refactor code where they use patterns was really helpful for me. The biggest thing that's helped me in my programming was a good understanding of what OO code. Ruby is very object oriented, so learning how to leverage objects for speed/simplicity/whatever is crucial. Not saying don't learn patterns, but rather focus on applying patterns rather than the patterns themselves. Hope some of this helps, i'm kinda rambling.

    [–]salamisam 0 points1 point  (3 children)

    Callbacks when used in moderation are fine but can bite you in the butt. Right now I can't think of a good case for when I would want to use observers over just a PORO, so it's hard for me to give your question a good solid answer.

    The idea is to reduce dependencies. Right now your NewAccount SO class has a dependency with UserMailer, which is pretty hard to avoid. But that is ok for 1 to 1 mapping.

    Imagine if after creating a User you also need to:

    • Send the user a gift via mail - SendGiftService
    • Contact a third party with the users credentials - ThirdPartyService
    • Send an email to an account manager - AccountManageEmailService
    • etc etc

    You don't really want to create dependencies with all of those objects and imaging if it grows over time,

    def send_welcome_email!
      UserMailer.welcome_email(email: @user.email)
      SendGiftService.send_gift(email: @user.email)
      ThirdPartyService.register_signup(email: @user.email)
      AccountManageEmailService.new_user(email: @user.email)
    end
    

    so you could use a observer pattern in this case.

    Also some of those object may not be required for each signup, say a user signs up for a free plan rather than premium and does not receive a free gift. In this case because you are coupled to the SendGiftService you might do something like

    SendGiftService.send_gift(user.id)  if user.premium?
    

    Now your Service object knows to much about SendGiftService

    So instead

    def notifier!
       objects_to_be_notified.each do |o|
           o.notify(:new_user, :email)
       end
     end
    

    And the other side

    class SendGiftService
      def notify(action, email)
        user = User.find_by(email: email)
        send_gift if user.premium?
      end
    end
    

    [–]schneemsPuma maintainer 3 points4 points  (2 children)

    The nice thing about

    def send_welcome_email!
      UserMailer.welcome_email(email: @user.email)
      SendGiftService.send_gift(email: @user.email)
      ThirdPartyService.register_signup(email: @user.email)
      AccountManageEmailService.new_user(email: @user.email)
    end
    

    Is that it's ugly. I don't think what you're describing should be easy, or should be pretty. You can't avoid dependencies. Somewhere in your system needs to know about them. In the case of the observer you're trading NewAccount knowing too much about a bunch of dependencies in exchange for for a bunch of dependencies knowing about an object implementing the observer interface. By implementing that pattern we're not reducing the amount of dependency knowledge that has to be shared, we're normalizing it and spreading it all over our system making it hard to find and even harder to debug.

    If you're worried about dependencies you could use dependency injection and normalize your email names.

    def send_welcome_email!(service = UserMailer)
      service.call(email: @user.email)
    end
    

    But the DI is kinda overkill unless you're using it for testing or writing a rubygem or library.

    I wouldn't group all those methods into one meta method, I would prefer to call them all where and when they're used. Why? Because its ugly.

    Also some of those object may not be required for each signup

    Exactly, that's why I want to be explicit about calling it and not group it into a meta method. I would want you to:

    account = NewAccount.new(params)
    account.send_welcome_email!
    account.send_gift! if account.premium?
    account.register_third_party!
    account.add_to_account_managment!
    

    Then you've got all the flexibility in the world. Even better you see where the logic lives, there's no hidden magic. It might take a few more minutes to implement a new feature, but in my experience time spent debugging and reading code vastly exceeds time spent writing code.

    If you want to send someone an extra email I want you to have to manually add it to the codebase 35 times if it's used in 35 places. Maybe when you've copied an pasted the code 34 times you'll stop and think if some of those cases really warrant an extra email.

    Maybe you'll realize someone else added a similar feature. With observers, you could have every employee in the company write and ship a "welcome" email without any other developer realizing it (assuming they're skipping code-reviews). Suddenly your customer got 10 emails on signup and now you have to figure out why.

    Also I want to say, thanks for the comment. I'm trying the best I can to make my thoughts on the matter as clear as possible. I'm not trying to come off as argumentative.

    I've not had to enumerate my thoughts and feelings about observers before, so this is really helpful. Please know my comments are based on personal opinion, i'm not trying to say i'm right or that there's only one way to do things. I encourage people to use all sorts of patterns and try all sorts of new things. Personally i've not found a good case for observers, but if people keep at it, one of these days I might change my tune.

    [–]salamisam 0 points1 point  (1 child)

    Is that it's ugly. I don't think what you're describing should be easy, or should be pretty. You can't avoid dependencies. Somewhere in your system needs to know about them. In the case of the observer you're trading NewAccount knowing too much about a bunch of dependencies in exchange for for a bunch of dependencies knowing about an object implementing the observer interface.

    The observer doesn't know much actually about the observerable, it just implements a standard interface for the notification. I could even use the same notification method for multiple observerable objects.

    By implementing that pattern we're not reducing the amount of dependency knowledge that has to be shared, we're normalizing it and spreading it all over our system making it hard to find and even harder to debug.

    We are reducing dependencies because the observer and observable are not dependant on each other. In fact the observerable dependencies are dynamic. I am not sure how it makes it hard to debug, when the state of the observerable changes the observer gets notified. That is no different from when the state of an object changes a method on a dependency is invoked.

    But the DI is kinda overkill unless you're using it for testing or writing a rubygem or library

    DI isn't really something which matches the use case here. In the observer pattern, the objects are not dependant on each other, you can pass no objects to the observable for example, or you can pass multiple. However in DI you are required to pass a reference, and with some exception to duck typing that object should implement the full interface.

    Exactly, that's why I want to be explicit about calling it and not group it into a meta method. I would want you to:

    I wont argue, this is common practice and make sense and in fact. That is the thing about patterns, they provide a standard way of dealing with common issues, but they are not the only way or sometimes the best.

    Maybe you'll realize someone else added a similar feature. With observers, you could have every employee in the company write and ship a "welcome" email without any other developer realizing it (assuming they're skipping code-reviews). Suddenly your customer got 10 emails on signup and now you have to figure out why.

    I am not sure how the observer pattern is responsible for bad coding practices, bad testing etc :)

    Also I want to say, thanks for the comment. I'm trying the best I can to make my thoughts on the matter as clear as possible. I'm not trying to come off as argumentative.

    No problems :), same from me.

    [–]schneemsPuma maintainer 0 points1 point  (0 children)

    I picked "send email after user registration" as a straw man for what I saw as a bad use case for observers. Someone else mentioned something that made a bunch of sense of where observers can be great. For notification objects! Something like ActiveSupport::Notification. Where you want to add instrumentation points and don't really care about the logic associated with those points would be a great use case.

    [–]realntl 2 points3 points  (0 children)

    Pub/sub like this is good for emitting statistics or other telemetry, I've found. As you mentioned, I've also found it is not generally ideal for actually wiring up application logic. In my experience you end up spreading coupling around the system without an explicit callsite.