Watch Out When Overriding Memoized Methods

This post was inspired by a 🐛 that was caused by not thinking enough when overriding a memoized method. Once we noticed the buggy behaviour, it took me some binding.pry time to figure out what was going on.

Lets check out what happened.

Code snippets

module Actions class FormAction attr_reader ... def initialize(...); ...; end def execute_action interpolate_properties form.process(form_attributes) end private def interpolate_properties interpolatable_attributes.each do |attribute| form_attributes[attribute] = interpolate(attribute) end end def interpolatable_attributes raise NotImplementedError end ... def form_attributes @form_attributes ||= action[:attributes].stringify_keys! end end end 

A simple Ruby class which is the base class for the specific actions we need to implement for our Rails app.

The action for which we figured out the bug was the action of creating comments on various objects. Its code looks like:

module Actions class CreateComment < FormAction private def interpolatable_attributes ['body'] end ... def form_attributes super.merge("#{commentable_type}_id": action_object.id) end end end 

Not much code is actually shown but just enough to spot the bug 🐛. Found it?

What was going on

Basically what was happening when running CreateComment#execute_action was the following:

In the execute_action we firstly need to interpolate some attributes - defined by the specific action class. In this case, for the CreateComment action that was only the 'body' attribute.

def execute_action # form_attributes = { 'body' => 'non interpolated' }  interpolate_properties form.process(form_attributes) end 

That's fine - we do some logic, the 'body' attribute gets interpolated and that value is set as the new value for the 'body' key in the form_attributes hash.

def execute_action interpolate_properties # form_attributes = { 'body' => 'interpolated' } form.process(form_attributes) end 

But, as the implementation of the CreateComment class uses super.merge(...) what we get is actually a new hash each time the method is called.

def execute_action interpolate_properties form.process(form_attributes) # process method called with { 'body' => 'non interpolated' } end 

The next time, after the interpolation is over, the method form_attributes is called when passing that hash to our form object and in that moment, the form object gets a newly generated hash that doesn't have the interpolated 'body' value. So the interpolation was actually pointless as would any other modification of the form_attributes hash prior to the form.process call be.

The solution 🐛🔨

module Actions class CreateComment < FormAction private ... def form_attributes @form_attributes ||= super.merge("#{commentable_type}_id": action_object.id) end end end 

Added memoization to the CreateComment#form_attributes method.

Simple as that and when you think about it, also pretty obvious that it needed to look like this from the start.

p.s. These snippets maybe look a bit too simple and memoization probably looks like not needed but it's a stripped version of the real classes. The form_attributes method is called and modified quite a few times 🙃