Dependency Injections Pros/Cons/Questions

gistfile1.md

Dependency Injection

Pros

• Classes are more modular, as they depend only on the interface of passed-in dependencies. Class behavior can be changed by swapping out a new component.
• Testing is simplified, since stubs can be substituted for any dependency.

Cons

• It's harder to understand how a class works when reading just that class. You may have to track down its invocation to see what kind of components are passed in.

Questions:

• Is DepedencyClass.expects(:new).returns(stub)) a smell, since we should have injected that stub to the class that uses it instead?
• If a class uses DI, should one only pass in already-instantiated dependencies, or is it okay to let the calling class instantiate them?
• Am I missing any pros or cons?

• I think stubbing a constructor is a smell but not an anti-pattern. If a collaborator is complicated enough that it was useful to stub methods on the collaborator, it's worth considering whether or not it would be useful to inject that dependency rather than making the collaborator's class a concrete dependency.
• By your second question, do you mean "is it wrong to pass in a class?" I'd say it's fine to pass in classes, as every class in Ruby is an object that can act as a factory. That being said, it's worth considering which component is in a better position to know what the arguments to the class's initializers should be: the class injecting the dependency, or the class that needs the dependency.

Not sure this makes any sense or is useful for your case, but in our travis codebase I found using "fat factories" useful: https://github.com/travis-ci/travis-build/blob/master/lib/travis/build/factory.rb#L7-19. These factories are (more or less) the only places where knowledge about classes and instantiation is centralized. The factories themselves are used through convenience methods as in https://github.com/travis-ci/travis-build/blob/master/lib/travis/build.rb#L39-41

Interesting stuff, Sven. I can definitely see how that sort of approach would make sense for objects with involved dependencies.

Ben - Re: "It's harder to understand how a class works when reading just that class." Kind of a blanket statement - You don't really define the alternative. Is the alternative having the code in the class instead of injecting it from the outside?

Even with injection, you could include an inner class that defines a "do nothing" default that provides some hints regarding a proper collaborator.

tuker,

"Is the alternative having the code in the class instead of injecting it from the outside?"

Yes. So imagine:

class Foo 
  def initialize(data)
    @data = data
   end

   def calculate_something
     Calculator.calculate_something(@data)
   end
end

vs.

class Foo 
  def initialize(data, calculator)
    @data = data
    @calculator = calculator
   end

   def calculate_something
     @calculator.calculate_something(@data)
   end
end

With the first, you know for sure what the calculator's class will be at runtime just by looking at the Foo class. With the second, you will have to look at the code that instantiates Foo to figure it out. I'd argue this is a con.

As for your comment on the 'do nothing' default, I'm not sure I follow. Can you show what you mean?

Right. So I'm saying that in the second example, when calculator is null, provide a default implementation that's right inside Foo. Then you get to inject -- but you also get to see an example of an impl.

Here are some thoughts I have after spending a minute reflecting on your points:

If a class uses DI, should one only pass in already-instantiated dependencies, or is it okay to let the calling class instantiate them?

I always make dependencies injectable, but providing sensible defaults. For example, if I have a method that does some calculations involving the current time, I instantiate a new object but only if the caller doesn't provide one.

def foo(current_time=Time.now)
  ...
end

I think providing defaults helps a lot with the point you listed against DI, since you know what kind of object is expected by using the default as an example.

Is DepedencyClass.expects(:new).returns(stub)) a smell, since we should have injected that stub to the class that uses it instead?

When taking a mockist approach to testing, I'm always facing the situation where I have to mock a dependency which is instantiated inside the object under test. I've reflected on the problem for some time, but never came up with a solution which I'm completely happy with. But I do think that stubbing the constructor of a class is a little odd, and wouldn't use that approach myself. Here's some other approaches that have worked for me:

• Inject factory object:

class School
  def initialize(options={})
    @enrollment_factory = options.fetch(:enrollment_factory) { Enrollment }
  end

  def enroll(student, course)
    @enrollment_factory.new_for(student, course)
  end
end

• Inject factory method:

class School
  def initialize(options={})
    @enrollment_source = options.fetch(:enrollment_source) { Enrollment.public_method(:new_for) }
  end

  def enroll(student, course)
    @enrollment_source.call(student, course)
  end
end

• Inject an instance or create a new one as default:

class School
  def enroll(student, course, options={})
    enrollment = options.fetch(:enrollment) { Enrollment.new }
    enrollment.tap do |enrollment|
      enrollment.student = student
      enrollment.course = course
    end
  end
end

• Stub complete creation method (this is test code only, doesn't involve changes in production code because, well, it's not really DI, but neither is stubbing new):

describe School do
  before do
    @enrollment = double
    Enrollment.stub(:new_for).and_return(@enrollment)
  end
  ...
end

These are some of the methods I came up with, there are probably some more that I didn't think of. Of course, all of those have pros and cons, and some are more suited to specific situations. Maybe this is a discussion for another gist, but since you touched this point I really wanted to share =)

Cheers,
Andre

I love IOC and DI, and have used the practice extensively in my C# days to make flexible systems that are easy to test and can be adapted to client requirements.

One thing I learned, however, is that overusing or misusing DI can lead to a "class explosion", where you have a myriad tiny classes that all do one small thing and only exist to be injected into the "real" objects of the system... It may end up proving difficult to figure out what the system in its entirety is trying to accomplish simply by looking at the classes. Further, the configuration of the system (think XML configuration in Spring) can end up being so opaque that when you want to adjust behavior, it's almost impossible to figure out where to start modifying the configuration.

Not making a case against DI, but those are some pitfalls I'd consider "cons".

tuker (and others) - I'm not sure I love the idea of providing a default value for the injected dependencies.

Since the default can be overridden, you still need to check the calling code to see whether it is. As such, you haven't addressed the issue of needing to check multiple files to be sure what's happening.

I suppose I'd prefer DI with a default specified to hard-coding a dependency though.

It depends on what you mean by: "See what's happening."

Surely we are imagining that there are multiple implementations that might be injected. By that reasoning, to check all "known" / in hand implementations, you're going to have to go to other files or inspect the tests.

But to "see what's happening" in the narrow compass of the implicit expectations of a basic implementation, a default implementation could help quite a bit.

I would guess that since we like small focused classes with limited responsibilities, any default implementation is not going to be a HUGE amount of code.

@r00k
I think what you're pointing against DI is a "problem" with duck-typing in general. You never know if some parameter you received is exactly what you where expecting, and the only way to "see what's happening" is checking the calling code and/or searching for the instantiation of such object.

The inappropriate quotes surrounding "problem" are due to the fact that it's not really a problem, just a trade-off made by the language designers.

@abernardes All of these abstractions and indirections are tradeoffs, duck typing and dependency injection included. They provide software that is easier to change overall and easier to understand at the unit level, but harder to understand as a whole system. Every class you abstract is another opportunity to name something and create a small, comprehensible unit, but it's also another hop the reader has to make to follow the path of the program. Flat, procedural programming is easy to follow but impossible to change, and the lack of abstractions and naming makes the "what" obvious but the "why" impossible to divine. In short, you need to find the right balance between abstracting yourself into a puzzle and creating a cemented mess of procedural glue.