1. 26

  2. 16

    This is a terrible solution to a bad misfeature. I see four things done by AR callbacks, in declining order of popularity:

    1. Mutate the model into a valid state before save (normalizing, setting defaults, decomposing attributes). AR::S doesn’t address this - which is good; generally these should never exist because they should be fired from custom attribute setters to keep the model valid.
    2. Mutate an associated model, bypassing validations “for performance”. Same as the previous, really, but reaching over code boundaries and doing it to some other object. Devs tell themselves they used update_attribute for performance, but most of the time they’re unknowingly persisting invalid data to drive an implicit state machine. AR::S doesn’t address this.
    3. Side effects like sending emails, clearing caches, and random twiddling of state in other systems. This is when AR callbacks are their most painful, and AR::S doesn’t address this at all. You could kludge them into an ActiveModel paradigm, but that time would be better spent rewriting the callback into something safe and updating outside code to use it.
    4. Instantiate or edit other AR models using a code path that would touch save!. AR::S nails this less-than 5% use case in the weirdest way possible, as the post critiques well.

    I think the correct solution looks more like removing AR callbacks, hiding the low-level unsafe interactions (update_attribute, save), and promoting a style that allows the dev to have some control over the lifecycles of their models.

    I guess I’ll never lack for work as a Rails consultant.

    1. 6

      All I could think when I was reading this was “holy shit, how can anybody think this is a good idea?”.

      I generally agree with your categorization of how people use callbacks, and the fact that this use case is a TINY fraction of use-cases. To even consider this being a good idea makes me frightened for what the code in Basecamp looks like. Every mature rails codebase I’ve seen minimizes use of callbacks period, but this seems to double-down on using them heavily.

      I’m gobsmacked, and I cannot imagine a scenario where I think it’s a good idea to use suppress

      1. 4

        My experiences agree with how you’ve seen AR callbacks used in the wild, although I’ll say I see #3 a lot more than #2, and almost as frequently as #1.

        The name of this feature definitely feels off, and my hunch is this: Along the lines of intention revealing names, when you see that some code is named “suppress”, your mind almost immediately asks “suppress what?”

        Then, because ActiveRecord does so freaking much, there is no single obvious answer for what action is being suppressed. I’m sure DHH disagrees with me, and that to him the most obvious action to suppress is save!, and hence why it does precisely that. However I can see loads of other people making different assumptions about what action is being suppressed, just based on the wide variety of what ActiveRecord does.

        I started writing a closing comment on my thoughts on callbacks, but stopped when it started to hit blog-like length… I’ll post it here if I get around to finishing it up. The short-and-sweet version is that any time callbacks come up conversationally, I put in my two cents and advise against using them. Ever. At all.

        Every use of a callback has an alternative that is superior in multiple ways.

        1. 2

          Yeah. I wrote the above comment and realized I could write 15-20 pages about the ways they’re (ab)used to misaddress the underlying needs.

      2. 4

        If your using Ruby and still require the Active Record pattern, I would strongly take a look at Sequel and Sequel::Model. If your in the position where you can dump the Active Record pattern entirely, Ruby Object Mapper is getting to be a very nice.