Skip to content

Instantly share code, notes, and snippets.

@blaix
Last active December 4, 2015 20:14
Show Gist options
  • Save blaix/e87da44be33d8b518470 to your computer and use it in GitHub Desktop.
Save blaix/e87da44be33d8b518470 to your computer and use it in GitHub Desktop.
An argument against an argument against DI

In this article, DHH uses this example:

def publish!
  self.update published_at: Time.now
end

saying DI folks would shiver at hard-coding Time.now and that they would inject a clock or something to more easily test it. He argues (accurately) that it's silly to make that more complex just for testing and you should do something like this:

Time.stub(:now) { Time.new(2012, 12, 24) }
article.publish!
assert_equal 24, article.published_at.day

I do shiver at hard-coding Time in the publish! method, but I also shiver at stubbing global, built-in classes like that...

I prefer to think about Dependency Inversion first, and then Dependency Injection if that's a way to accomplish it.

So when I see this:

def publish!
  self.update published_at: Time.now
end

I do shiver, because hard-coding like that is well established to be a code smell. Now, a smell does not mean it's wrong in every case, but it means I should examine this. I'm not even thinking about tests right now, I'm thinking about inverting this dependency. Here's how I'd do it:

def publish!(time)
  self.update published_at: time
end

I've inverted the dependency. No dependency injection framework needed. And no stubbing globals or bringing in complex gems (like Timecop) to test it:

article.publish! Time.new(2012, 12, 24)
assert_equal 24, article.published_at.day

The fact that it's easier to test wasn't the goal, it was a side effect of a better design. Now the system is ready to allow users to define publish dates in the future for example.

And if you miss the old API? Just give it a default:

def publish!(time = Time.now)
  self.update published_at: time
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment