I am starting a series of in depth posts about refactoring Rails/Ruby so follow along if you want to learn more on how I keep my code clean, and sane.

Callbacks are double edge swords. They are predictable actions that always executes. And can (most times) be evil, especially when trying to do a simple update and suddenly something else gets changed because of a callback.

Callbacks should be simple, and should not be intrusive. When you find yourself calling model.save(validate: false) to avoid a callback, it is time you look into refactoring.

Some less evil examples are:

Sending email when creating a user (still a pretty evil)

Adding timestamp when a boolean changed.

Generating a user token on create.

The Code

Enough explanation and let’s jump into some code.

Working at Bluethumb, we deal with a lot of images and need to extract the closest color of an artwork for filtering and pre ActiveStorage, we have a model for storing artwork images call (of course) Attachments.

class Attachment < ApplicationRecord

belongs_to :artwork, touch: true

mount_uploader ..... after_save :extract_color! def extract_color!

# Processes image and extracts colors

Attachment::ColorExtractionJob.perform_later(self)

end

end

As you can see, it’s a pretty harmless callback. Every time it is updated, image changed, it extracts the dominant color and saves it to Attachment#color .

Now notice one thing,

class Attachment::ColorExtractionJob < ApplicationJob

queue_as :low_priority def perform(attachment)

attachment.color = DominantColorGetterService.new(

attachment.file

).call

attachment.save(validate: false)

end

end

You should already notice the code smell. Calling validate: false just to avoid extracting color again when saving is bad. Now you can’t trust yourself saving it without triggering any side-effects.

Just imagine this callback sends an email to your customers and got through test and reviews. SH*T!

Duplicate explicit .save to the Rescue

It is always best to think of your models as pure components so you can feel safe when performing updates and mutations without triggering side effects.

I can always bring up a new Service class but lets not go there yet.

Don’t be afraid to unDRY, more code does not make your code hard to reason with, it makes things much more explicit and clear.

class Attachment < ApplicationRecord

.... # after_save :extract_color!



# def extract_color!

# Processes image and extracts colors

# Attachment::ColorExtractionJob.perform_later(self)

# end



def save_and_extract_color

save.tap do |success|

Attachment::ColorExtractionJob.perform_later(self) if success

end

end

end # .tap is just shorthand to return the object eg. def do_something

success = save

do_something_else if success

return success

end

You can read more about #tap here

Now you just have to replace everywhere you call .save with .save_and_extract_color when you need to extract color after saving, damn! But that’s the small cost of writing safer, sane models.

AND I can safely use attachment.save without having to call validate: false , that’s a huge win in my book.

Now go out there and write safer models, your team and future you will thank you.