Skip to content

Instantly share code, notes, and snippets.

@richmolj
Last active December 18, 2015 16:59
Show Gist options
  • Save richmolj/5815196 to your computer and use it in GitHub Desktop.
Save richmolj/5815196 to your computer and use it in GitHub Desktop.
Back and forth with Lou

Lee:

I didn't want to start derailing the thread, but I wrote two gists that night I wanted to get your personal opinion on before posting/avoiding posting.

Without DI: https://gist.github.com/richmolj/5811237

With DI https://gist.github.com/richmolj/5811358

Does this accurately display the additional complexity added by DI, or is the example unfair? Not particularly trying to make a point here, more trying to better orient myself to the opposing mindset.

Lou:

Yeah, I think your examples are pretty fair. There certainly is some overhead for going the DI route, I definitely wouldn't dispute it, but to me the extra complexity in the constructor pays off more times then not.

When you want to reuse the post somewhere else, but have the stats collected differently, you just pass in a different collaborator. In other words, I think this helps with the open/closed principle, which is a bigger deal than the enhanced testability (there, I agree with your previous assessment, It's a minor plus, but it wouldn't be the main reason I'd want the DI code--though I'm sure others would disagree with me). If we are getting into the world where we're trying to aim for more code reuse, we want to minimize the number of instances where we actually have to change class implementation, because this is a much greater risk of introducing regressions then building up the behavior you want from small existing pieces.

Also, you could minimize the pain of that constructor by either the parameter defaults or a factory etc. In the example I'd probably just put a factory method on the post class, like Post.simple_post, that wraps the last three lines. In general, I'd prefer the more flexible interface and then work with it via a facade of some sort. This keeps us from monkey patching/changing internals of classes that are used in many places in production code.

So, yeah, I would definitely add it to the discussion and see how folks feel about it. I was thinking about this a lot last night, and I think you're right that folks have different tolerances for the number of abstractions they want to deal with. I think I might have preference for more generalized stuff, just because I tend to look at code more from and abstract math perspective than an engineering one. YMMV. Would be interesting to see how everybody else feels.

Though in either case I think we should take a look at how we do (or don't do) documentation. That helps a lot in nailing down these abstractions with concrete details about the contracts.

@richmolj
Copy link
Author

I've tried to be clear I don't personally see DI as an all-or-nothing scenario. I'll refer to the Post @backend example in my original gist, and the 'progression' gist in my last comment.

I do think "always DI" has been suggested. I've been told in this situation:

https://gist.github.com/richmolj/5823903

We should be injecting Encrypter, even if there is no other encryption class. And the consensus for this:

https://gist.github.com/richmolj/5824046

Seems to be the DI approach, even if Comment and Photo are the only possible repositories (there's a slight caveat here that we could say 'there is a separate repository when the "database" is down' but for the moment I'd like to treat this as a tangent since I've heard the same thing regardless of this situation.

In my 'progression' example (https://gist.github.com/richmolj/5822934), I've been told I should not be progressing, I should immediately be injecting the clock, in case at some point in the future I don't want Time.now

These all seem 'Full-DI' to me, since I never have an actual need for it. The logic is, 'at some point you may have a need for it, so start with it'

The example at the top was an attempt to distill this into one, quickly readable example of my central point that:

A) There is a tradeoff between abstraction and simplicity, here
B) There are alternatives before going full-DI (like Service Locator)
C) Still, go DI when you need.

If you believe the example doesn't make this point clearly, let's pair to create a better example (or disagree with the fundamentals of my assertion).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment