1

The general approach to DI that I see in answers like So Singletons are bad, then what? encourages business objects that collaborate with other objects to (a) not directly create those instances and (b) have them passed in at construction. I can understand (a), but not (b). This seems to occur most often in response to overuse of Singletons. But why not just have a modified approach to singleton:

class SingleInstance {
  virtual foo();
  virtual bar();

  static SingleInstance getInstance() {
    if(instance_ == null) {
      instance_ = new SingleInstance();
    }
    return instance_;
  }

  void setMockInstance(SingleInstance s) {
    assert(instance_ == null);
    instance_ = s;
  }

  static SingleInstance instance_;
}

So now this is no longer a Singleton with a capital S (per Misko Hevery) but in this case still enforces one instance. All code that wants to access the single instance can still call S::getInstance() without cluttering up their constructors by explicitly requiring the instance to be passed in. The default can still be still lazily initialized in the production code, but for test can still be mocked.

In the referenced answer the first benefit of DI is listed as:

It makes the code easier to read; you can clearly understand from
the interfaces exactly what data the dependent classes depend on.

But why is that not a violation of encapsulation? Do I really need to know from the public interface everything that accesses the SingleInstance/Database/etc?

Assume you have a Database and 30 TableGateway classes responsible for CRUD operations on those tables. In the DI approach TableGateway constructors would accept the Database on in its constructor. Then a business logic class would accept the tables it collaborates with/uses:

class BusinessLogic {
  BusinessLogic(Table1 t1, Table2 t2, Table3 t3);
  void doBusiness() {
    t1_.query(...);
    t2_.insert(...);
    t3_.update(...);
  }
}

How is that churn of explicit dependencies in the constructor advisable?

  • 1
    Are you asking about "churn of dependencies" (whatever that is)? Whether the answer referenced encourages an encapsulation violation? Whether the answer referenced only has to do with singletons? Something about passing things in to constructors? What is the question? – Michael Shaw Oct 08 '14 at 00:49
  • Specifically: "Why does dependency injection encourage collaboration to be exposed via constructors?". The "churn of dependencies" points out what looks like a drawback - i.e. to change the collaborators required by an implementation requires rework/churn of public api. – Daniel Davidson Oct 08 '14 at 14:32

3 Answers3

3

Static data (including singleton instances) is problematic for a number of reasons. It's not thread-friendly. You have added a method to set the single instance to a mock instance. Presumably that is done to support unit testing. This will fail if tests are run in parallel and require different mocks for your Singleton. Worse, the tests will fail when run together but will pass when run separately.

Even without running tests in parallel, static data introduces a global state that is retained between tests. A test that depends on static data could pass reliably when run alone against the initial global state, but consistently fail if run after another test that modifies the global state.

For these reasons, mutable static data should be avoided. I have seen many more incorrect uses of mutable static data than correct uses.

kevin cline
  • 33,670
  • What you say about static data is true. But I think most difficulty comes from shared state which static data implies. For the non-parallel single test run, going from singletons to lots of objects sharing their collaborators via constructor does not change the object graph at all. It is still shared mutable state and read/write issues - both single and multithreaded will still be an present. It seems the only real changes are - it lifts the restriction of one global/systemwide instance and require (heavy?) design-time demands on constructors. – Daniel Davidson Oct 08 '14 at 14:18
  • 2
    @Daniel: Injected dependencies need not be and should not be shared by different unit test classes. Each unit test class should construct and inject new mock objects needed by the class being tested. – kevin cline Oct 08 '14 at 15:53
  • Thanks. I can't yet up-vote. I was referring to "non-parallel single test run". A DB/TableGateway is shared mutable source/sink. The focus on testing is understandable and what I'm trying to grok. I don't know your mapping of unit test class to unit test. But if you test a business object that reads/writes/reads/verifies a DB interaction the same sharing in the test is required as in the system. I now see benefit of DI allowing same code run simultaneously/independently as multiple systems. But it does test scenarios production by design won't see. – Daniel Davidson Oct 08 '14 at 16:40
2

The reason I give dependencies to constructors is:

  1. Most of the time the dependencies are required, so why not give them to the constructor
  2. Giving them to the object in an other way makes using them a bit more complex, since you have to make sure you have the dependency

Of course this doesn't really count when you are using a framework that controls the dependency injection, because the framework will make sure all the dependencies are set before returning the requested object.

  • The issue is not if dependencies are required it is who needs to know. If you sneak a single instance to a user when needed, the dependency is hidden. A form of hiding with benefits and drawbacks.

  • getInstance() seems pretty simple. Knowing up front everything you need use and hopefully ever will ever need to use in a business object (to avoid API churn) seems complex.

  • – Daniel Davidson Oct 08 '14 at 14:01
  • Can you elaborate on the last point? BTW: I'm using C++, so maybe the benefits are greater in languages with more reflection? – Daniel Davidson Oct 08 '14 at 14:57
  • If you wanted answers specific to c++ you should have added the c++ tag to the question. – kevin cline Oct 08 '14 at 15:55