Dependency Injection – Is Singleton with Dependency Injection a Good Practice?

dependency-injectiondependency-managementsingleton

It's a good idea to have a Singleton implementation with dependency injection?
I have some classes that performs some heavy tasks on instantiation (related to database loading).
I would like to use them always as new instances but is problematic due to performance.
The code I am working with is from a legacy app, so it's preferible to dont touch it too much.

My question is:
Can I create singletons as this example?

IMPORTANT: consider de dependency is kind of a logger, so i don´t have multiple implementations of it, only one.
Is not a problem if all the instances shares the same dependency instantiation.

I am interested in using interfaces as they make my solution less coupled between projects.

public class HeavyTaskDb()
{
   private static HeavyTaskDb instance;
   private IDependency dependency;

   private HeavyTaskDb(IDependency dependency)
   {
      this.dependency=dependency;
   }

   public static HeavyTaskDb Instance(IDependency dependency)
   {  
     if(instance==null)
     {
          instance=new HeavyTaskDb (dependency);}
     }
     return instance;
   }
}

Best Answer

This is problematic, because if your program calls 

    HeavyTaskDb.Instance(X)

first, and then 

    HeavyTaskDb.Instance(Y)

later (maybe in a completely different area of the code base), it returns an object of type HeavyTaskDb initialized with X, and not as expected with Y.

The best way to avoid this is probably not to use the singleton pattern at all, do the one-time initialization containing htdb=new HeavyTaskDb(X) in one defined placed and make the htdb object available to all scopes where it is needed (maybe by passing it around, maybe by injecting it into the using classes, whatever makes most sense in the related context).

If your really think you need to have a global variable of type HeavyTaskDb in your program (just like your singleton is a global variable), then consider to implement it this way:

public class HeavyTaskDb()
{
   private static HeavyTaskDb instance;
   private IDependency dependency;

   private HeavyTaskDb(IDependency dependency)
   {
      this.dependency=dependency;
   }
   // call this somewhere at the beginning of the program,
   // before Instance get called
   public static void InitInstance(IDependency dependency)
   {
         instance=new HeavyTaskDb (dependency);
   }

   public static HeavyTaskDb Instance()
   {  
     if(instance==null)
     {
          throw new Exception("instance was forgotten to be initialized")
     }
     return instance;
   }
}

This is still not an ideal solution, but at least the caller of HeavyTaskDb.Instance is not fooled about what he gets.

Related Topic