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 8 '14 at 0: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 8 '14 at 14:32
2

Your modified approach to singleton is still a singleton, and has most of the drawbacks of a singleton:

  • It still introduces high coupling across the application - that is, if one day you want to refactor away from singleton, you will have a hard time doing it;
  • It still gives a way for any component to collaborate with the singleton - meaning that potentially, the singleton can communicate with any piece of the system - but if you go for any kind of layered architectural pattern (MVC, MVVM, 3-tier), then the singleton is a point that has to be controlled.

Encapsulation is related to the fact that an object exposes the operations it supports without requiring that clients know about its implementation - but when you invoke a constructor, the object does not exist yet. The constructor is an implementation detail of the object, equivalent to a static method. In other words, once the object is created, the constructor arguments are irrelevant to the operation of the object.

As for the "churn" of explicit dependencies, making the dependencies explicit is one of the goals of constructor injection. If you have too many, it is taken as a sign that a refactoring, or a reorganization, is in order. (E.g. group related tables into a logical group).

  • First bullet point: yes, but how is the alternative BusinessObject(Colloaborator1, Collaborator2, ...) not high/higher coupling. I don't see how you are in a better position when it is time to refactor with this scheme. Second Point: Makes sense. There is a sense, though, that the policing mechanism to control who has access to the instances of cardinality of 1 is to make it difficult to use (i.e. require it via ctor). Effective, but is it desirable? BTW: I'm focusing now on C++ so reflective and automated testing frameworks are not in the toolbox. – Daniel Davidson Oct 8 '14 at 14:28
  • 1
    With singleton, client code specifies that it gets the instance of the object. With dependency injection, client code only specifies that it needs an instance of the object. As a trivial example, dependency injection allows me to refactor away from singleton easily. E.g. if I have a hash map that represents my configuration properties, it is easy with DI to change from a unique instance to passing (e.g.) a filtered view of the properties that will differ for each object. Or it is easy to go from a "single object per process" to "single object per task". – jhominal Oct 8 '14 at 14:47
  • 1
    The parts about "constructor is an implementation detail" is very helpful. It seems the newer languages with toolkits/libraries are pulling all of that construction logic out and doing it upfront and then pushing the object graphs created up front through the system. Maybe the tools of the newer languages make the difference. In C++ it seems tedious. I'm trying to see if it is worth ridding some C++ database code of singletons and it looks like a lot of work. It might be worth the DI for testing, but only if you could successfully mock a complete database. – Daniel Davidson Oct 8 '14 at 14:48
  • 1
    @DanielDavidson: I will admit that I speak from a C#/Java perspective. Mocking a complete database is generally a waste of time (if only because you will never have the same SQL parser) - my recommendation for testing business code that accesses a database would be to encapsulate database access behind repository objects, the main role of which is to define functions that correspond to the queries executed on the database. Unit testing these repository objects can be done by connecting to a small, isolated database. – jhominal Oct 8 '14 at 14:57
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.

  • 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 8 '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 8 '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 8 '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.

  • 1) 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. 2) 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 8 '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 8 '14 at 14:57
  • If you wanted answers specific to c++ you should have added the c++ tag to the question. – kevin cline Oct 8 '14 at 15:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.