Code Smell of the Day: "Threadsafe" Singleton

gistfile1.md

Example

class PonyService
  def self.instance
    Thread.current['pony_service_instance'] ||= new
  end
end

PonyService.instance.find params[:pony_id]

Why It's A Smell

Here's how this anti-pattern usually arises: A service object is created and made a singleton because you don't need more than one, right? Well, it turns out that this shared service object isn't threadsafe, so you bolt on threadsafety using a thread-local variable.

How You Fix It

Stop using the singleton pattern. Instead of taking a leaky abstraction and patching up the leaks, sometimes it's best to re-evaluate your reason for using the abstraction in the first place. In almost all cases, you can just do this:

PonyService.new.find params[:pony_id]

Object instantiation is cheap. Besides, you're using an object oriented langauge so you shouldn't be afraid to create objects.