The other day at work I run into a bug that I thought it would be worth documenting. The root of the bug was modifying one of the arguments in a method. I have always read how bad is to mutate the arguments, but I would say that this is the first time that this problem hits me in a painful and hard to find way.

Some context: I had some data stored in Memcache and I wanted to: access it, decorate it (e.g. convert plain numbers into money formatted numbers) and present it to the caller. The plan was simple:

1. Create a DataCacheManager that will be responsible to read and write from the cache.
2. Create a DataPresenter that will be given a Hash that has been fetched from the cache, and decorate some of the entries and leave the rest how they are.

Here is what I was doing (simplified):

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

class DataCacheManager
  # ...
  def fetch
    Rails.cache.read(cache_key)
  end
  # ...
end

class DataPresenter
  def initialize(data)
    @data = data
  end

  def present
    data[:profit] = decorate_profit

    data
  end

  private

  def decorate_profit
    '$' + data[:profit].to_s
  end
end

class StatisticsInteractor
  def execute
    # Fetch data
    data = DataCacheManager.new(...).fetch # This would return a hash that looked like this: {units: 3, profit: 4}
    DataPresenter.present(data)
  end
end

Problem

Can you spot the problem here? Well, the main problem is in DataPresenter#present we are overwritting data. Somebody has trusted us giving a reference to an object, and we are overwritting the data contained within that object. So, the first time that I fetched the cache, I would received a plain number (e.g. 8), but the second time I would get a decorated number (e.g $8).

What I shown is a simplified version, but let me tell you that at the moment that I saw some dollars in the cache, I was pretty confused on what was going on. It took me a while to realize that the problem was that I was mutating that data.

Solution

Instead of overwritting data, I duplicated it in the initializer of the Decorator class. I wanted to be able to just decorate some of the keys that were being given to me, and not touch the other ones, but at the same time I didn’t want to mutate whatever object was given. So here is how it ended up looking:

1
2
3

def initialize(data)
  @data = data.dup
end

This was definitely a good learning experience. If we want to use any object that is given to us, DO NOT mutate it. The caller should trust that we are good people and are not going to mess with the data that they have given to us.