Java Object creation pattern and design

Question

I have recently started working on a Java project that is already with a sizeable codebase developed by a team over 3 months. I noticed that at many places , some objects they are instantiating directly in the constructor of the client object , rather than using a dependency injection. I wanted to refactor the object construction into a factory and use some injection framework.

I have created a factory that essentially is a one liner for a doing new <type(some params here)>. There is nothing fancy here - no singleton , no static factory pattern. Just a newInstance() method that returns a new instance of the dependency.

To show something in code :

class A {   A() {  
       B bobj = new B();  // A and B are coupled directly
    } 
}

I want to refactor this to :

BFactory {
    newInstance() {  return new B(); // return B implementation  }
}

 class A {
   A(BFactory factory){  
     B bobj = factory.newInstance(); // A does not know about B impl
  }
}

My argument is that objects should not be created anywhere in the code except in a Factory meant for that purpose. This promotes loose coupling , otherwise you are coupling the two types tightly. One senior member ( the author of the code I am trying to refactor ) feels that the one liner factory is a over-complicating design.

Are there authoritative advices/references on patterns governing this problem ? Something that can be used to decide which approach is better and why exactly ?

Answer 1

One senior member ( the author of the code I am trying to refactor ) feels that the one liner factory is a over-complicating design.

This looks like the crux of your question and not whether you should be refactoring the code or not so let us answer it rather than deviating from the actual question. If we consider the examples that you present in your code, I agree with your colleague. You shouldn't be creating a factory class for each dependency you want to inject. There is nothing wrong with what you are trying to achieve but the way you try to achieve it is an overkill.

You either depend upon a hierarchy of Factory classes that know how to create each and every dependency or you depend on the actual class itself and have a Container that can wire the objects together for you.

Option 1 : Depend on a common Factory

class A {
   B bobj;
   C cobj;
   A(Factory factory){  
     bobj = factory.createB(); 
     cobj = factory.createC();
   }
}

Option 2 : Depend on the dependency directly

class A {
   B bobj;
   C cobj;
   A(A a,B b) {  
      this.bobj = b;
      this.cobj = c
   }
}

You can then create a Container class that knows how to wire objects together :

class Container {
    public static B createB() {
        return new BImpl();
    }

    public static C createC() {
         return new CImpl();
    }

    public static A createA() {
        return newAImpl(createB(),createC());
    }
}

The examples presented above are way too basic. In the real world, you will mostly have a more complex graph of dependencies. That's where DI frameworks come in handy instead of reinventing the wheel. If your ultimate goal is to start using a DI framework, you could go with option 2 since DI frameworks achieve inversion of control by supplying dependencies to their clients rather than the client code asking for them.

Answer 2

Your underlying point is perfectly valid, it's usually not a good idea to instantiate objects directly in the constructor (they may be valid exceptions from that rule).

If you do this:

class Car {

   private Engine engine;

   public Car() {
       engine = new Engine();
   }

}

You will have a hard time testing the Car without the engine. You'll have to use reflection in order to exchange the instance by a mock.

If you do the following instead

class Car {

   private Engine engine;

   @Inject
   public Car(Engine engine) {
       this.engine = engine;
   }

}

it is very easy to test the Car with a fake engine, replace the implementation of engine or change the way the engine should be constructed (e.g. add more parameters to the constructor).

But you should definitely use an established dependency injection framework instead of writing your own factories. If you pass in a "common factory" as Chetan suggested you end up hiding your dependencies.

Good resources for more motivation using dependency injection can be found here: https://github.com/google/guice/wiki/Motivation or https://youtu.be/acjvKJiOvXw (very good talk, should be worth your time).