4

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;
   }
}
  • Do you use any DI containers? – kayess Sep 27 '17 at 9:14
  • No, is not planned at the moment – Badulake Sep 27 '17 at 9:25
  • 2
    What kind of problems do you see in your current proposed solution? – larsbe Sep 27 '17 at 10:50
  • it's my question, is this dangerous? or a bad practice? – Badulake Sep 27 '17 at 11:15
  • 3
    It doesn't seem any worse than using a Singleton to start with (which many would consider an anti-pattern itself). – Sean Burton Sep 27 '17 at 11:51
9

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.

  • ok, many thanks. The code is inside a library for 3rd party compnaies , so i cannot manager very well where to initialize the singleton. Do you have any idea? – Badulake Sep 28 '17 at 5:47
  • @Badulake: calling the initialization is the reponsibility of the calling code, an I don't have a crystal ball how that code is structured. – Doc Brown Sep 28 '17 at 19:17
  • go to the future and tell me that – Badulake Sep 28 '17 at 19:28
  • 1
    @Badulake: I think I have to repair that flux capacitor first. – Doc Brown Sep 28 '17 at 19:33

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.