Why We Should Inject Dependencies
5 points by PuercoPop
5 points by PuercoPop
One of the things that I find more interesting from Hanakai is their approach to dependency injection. dry-container+dry-autoject is the first dependency injection framework that looked aligned with Ruby values 'values'.
Most of the time one doesn't need to do dependency injection in Ruby, and keyword arguments + memoization can be used in most of the cases where I would want to reach for DI. This post explains the perspective dry-container and friends are coming from.
How they divide arguments to the class vs to the method is a little different to how I normally write Ruby but it is a divide that I can make sense of. In the spectrum of what is a dependency there are several answers. From the infra perspective it might be a 3rd party service (Postgres, Redis, MailChimp, etc). A maximalist view in the language perspective might be that collaborator classes are a dependency [in the global namespace] and that is the perspective where the proposed divide is coming from.
The way I would regularly write the class would be to inline some dependencies like UserRepository, which I normally don't try to substitute during tests. And things like Confirmation mailer would be a memoized method which we can override with a keyword argument during tests.
class UserRegistration
def initialize(email, password, confirmation:); ...; end
def confirmation; @comfirmation ||= ConfirmationMailer.new
The resulting service class might look a little unusual at first, but the design is more testable without resorting to things like method stubs.
I don't understand why this is a yes or no question to people. Either you don't need it, or you do and when that answer changes for that particular case you change it then.
Dependency injection is an incredibly powerful tool! But I have deep suspicions around systematizing it in this manner. While I haven't written any Hanami, I have a lot of experience with this pattern in the .NET world, and I think it has a pretty disastrous effect on teams there.
Your DI container tends to end up massive and starts containing business logic that should belong elsewhere. Your example is a great one of what I'd be hesitant to do, actually!
container.register :confirmation do
if Enabled?(:notification_service)
NotificationService.new
else
ConfirmationMailer.new
end
end
Here you've hide away a pretty core decision (push notifications vs emails, I think) that your application makes in a DI container. Instead, a class relevant to sending confirmations should be making that decision.
Here this looks like a feature flag that may be enabled at startup. This pattern gets a lot worse once you start needing runtime dependency decisions. I've seen teams cram some truly awful code into DI containers to achieve that.
DI containers often become an obsession too. DI is a tool, and you should not use a single tool in every place, but these frameworks really encourage that all-or-nothing mentality. Sometimes you do in fact need to just write new! And DI containers really discourage that and limit the kinds of solutions people reach to when designing a system.
A good example of this is your example only has singleton objects, and I think DI containers really push people towards singleton objects. They're useful, but when they're the only solution you see, you get can some really messy code.
DI does let you achieve more testable units of code, but I think you've misread the linked article on testing behavior, not implementation.
From that article:
tests should focus on testing your code's public API, and your code's implementation details shouldn't need to be exposed to tests.
The UserRegistration registration class is likely not your public API! In a web application, your HTTP endpoints are! UserRegistration is an internal class that you've now coupled to a test, making it harder to change.
In short, I think it's ok to have hardcoded dependencies sometimes, and it's ok to just new and pass some arguments sometimes.
(note that this comment comes from a lot of .NET trauma specifically. if something isn't accurate to Hanami, sorry!)
(I'm the author of this document)
The point is that breaking the identity between Class name and Instance give you freedom you didn't have.
Let's say every caller was instantiating ConfirmationMailer and you want to introduce NotificationService. The naive approach is add a branch to every caller. (Yes, I've seen this)
Refactoring as a new class is reasonable, but now every caller needs to be updated to use a different thing. What this tells us is that knowing how to build the thing is a distinct responsibility, and it has been distributed throughout the code piecemeal.
If everything is addressable as a container key by default, that means this identity is severable from the Class even if it begins as a simple 1-1 mapping. This means you could introduce this new class by changing what the key returns, and as long as the contract isn't broken you don't have to touch anything else.
I've seen teams cram some truly awful code into DI containers
Counterpoint: the awful code was contained into one place, rather than sprinkled throughout the project. That's still a win.
DI is a tool, and you should not use a single tool in every place, but these frameworks really encourage that all-or-nothing mentality. Sometimes you do in fact need to just write new! And DI containers really discourage that and limit the kinds of solutions people reach to when designing a system.
The Dry-System approach to DI is more like falling into the pit of success. Nothing is made impossible. Don't want a singleton? memoize: false. Don't want to register the class as a key? auto_register: false. There is no boilerplate involved in wiring things up, you just reference key names by convention. The only time you have to write DI wireup code is for exceptional cases.
The UserRegistration registration class is likely not your public API! In a web application, your HTTP endpoints are! UserRegistration is an internal class that you've now coupled to a test, making it harder to change.
This suggests that unit testing should never take place, but I think that is a puzzling interpretation of the article which specifically uses unit testing of a class as the central example. You're not wrong that the HTTP API is public interface, but the meaning of "public interface" is contextual.
Ah, lemme make something clear: I do think DI is a great tool. And your example of it is something I 100% agree with. I think that DI containers are the mistake.
Counterpoint: the awful code was contained into one place, rather than sprinkled throughout the project. That's still a win.
I think awful code in one place can be good code in another.
The Dry-System approach to DI is more like falling into the pit of success. Nothing is made impossible. Don't want a singleton? memoize: false. Don't want to register the class as a key? auto_register: false. There is no boilerplate involved in wiring things up, you just reference key names by convention. The only time you have to write DI wireup code is for exceptional cases.
It definitely encourages some standard behaviors, but I don't think those are successful ones to make defaults. The default for OOP shouldn't be that initialize is solely dependent classes. Sometimes you do need to pass in things to construct an object!
This suggests that unit testing should never take place
I am suggesting that! I know that's a hot take though. I think that's what the article you linked was also arguing for though!
You've got me thinking a bunch though, and I appreciate that!
I think that DI containers are the mistake.
I explicitly demonstrated a gradual movement from hand-coded initialize to DSL to DI Container as a spectrum, but that said I have yet to encounter a downside to using containers but I can't say the same for the other options.
I think awful code in one place can be good code in another.
We go to production with the codebase we have, not the codebase we want to have. I think you are thinking too abstractly. What I'm getting at is how to structure our systems in a way that allow for gradual change without shotgun surgery. Coupling class identity to objects we use is not in fact necessary.
The default for OOP shouldn't be that initialize is solely dependent classes. Sometimes you do need to pass in things to construct an object!
Nothing about my argument is class-focused. The rubric for dependencies is only about whether the thing changes per-request or not; whether it happens to be a class or something else is an implementation detail. A container key can be anything.