-
-
Save richmolj/5822934 to your computer and use it in GitHub Desktop.
# First iteration | |
class Post | |
def publish | |
self.published_at = Time.now | |
save | |
end | |
end | |
# Second iteration | |
# published_at logic gets more complex | |
class PublishTime | |
def initialize(special_timing) | |
@special_timing = special_timing | |
end | |
def stamp | |
if @special_timing | |
Time.now * 100 - 365 | |
else | |
Time.now | |
end | |
end | |
end | |
class Post | |
def publish | |
publish_time = PublishTime.new(self.special_timing?) | |
self.published_at = publish_time.stamp | |
save | |
end | |
end | |
# Iteration 3 - lots of different timings | |
# We figure out the strategy at runtime | |
# DI! | |
class Post | |
def publish(time = PublishTime.new) | |
time.special_timing = self.special_timing? | |
self.published_at = time | |
save | |
end | |
end | |
# PublishTimeFactory: wysiwyg ok? | |
class PostController | |
def create | |
post = Post.new(params[:post]) | |
PublishTimeFactory.for(post, rand(9)) | |
post.publish(time) | |
end | |
end |
I don't think anything is tied to the database, I don't show #save's implementation and I'm never specifying ActiveRecord.
I wouldn't call a DI or non-DI solution better or worse in this case. Still, we can change this example to pass something other than a boolean tied to post. I think the central point still I was making works in any case, or no?
I don't think anything is tied to the database, I don't show #save's implementation and I'm never specifying ActiveRecord.
Sure, I guess that's just what I assumed based on the fact that there was no implementation shown for Post#special_timing?
. But I think the point still stands, if that was coming from the database, then this would be an example where I would not use DI. I'm not sure if that's helpful.
Still, we can change this example to pass something other than a boolean tied to post. I think the central point still I was making works in any case, or no?
I guess the central point I'm trying to make is that DI is just a tool. The important thing is that we encourage code that is easily extensible by adding additional objects/classes not by modifying ones that already exist. Especially as we endeavor to make code reuse more of a goal. If we don't we're going to be walking into a whole boatful of prod errors.
So two points on this example--tldr, I wouldn't use DI in this instance.
I'll get to that in a second; but, first I would comment that I find passing a boolean value to another method as a parameter is a code smell, and that nine times out of ten it signifies control coupling that we want to remove. Not sure if that's just a consequence of creating an easily grokable example, but to me it means that there's some polymorphism hidin' in them there hills. Let's start digging:
So what we're really talking about here isn't behavior based on the incoming request, it's based on some data stored in the model. So rather than have a boolean, let's future proof the code and add something we can use to get the right type of polymorphic
Publisher
object. This is not DI, but I guess it's still IoC? I guess that might be even worse than DI to some, but meh, I'm happy with the above code.Anyway, the reason I wouldn't use DI here and pass in the collaborator to the
Post
? Because the thing we're using is already coupled to the database schema. The database is going to tell us which thing to create. It's part ofPost
already, so injecting it would mean digging into Post to get the type, and then passing it back in. Perfect example of when DI actually doesn't buy you anything.I think this case is rare though. And in most cases, you'd want to do DI.