7

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;
   }
}
7
  • Do you use any DI containers?
    – kayess
    Commented Sep 27, 2017 at 9:14
  • 1
    No, is not planned at the moment
    – X.Otano
    Commented Sep 27, 2017 at 9:25
  • 2
    What kind of problems do you see in your current proposed solution?
    – larsbe
    Commented Sep 27, 2017 at 10:50
  • it's my question, is this dangerous? or a bad practice?
    – X.Otano
    Commented Sep 27, 2017 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). Commented Sep 27, 2017 at 11:51

1 Answer 1

10

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.

4
  • 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?
    – X.Otano
    Commented Sep 28, 2017 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
    Commented Sep 28, 2017 at 19:17
  • go to the future and tell me that
    – X.Otano
    Commented Sep 28, 2017 at 19:28
  • 1
    @Badulake: I think I have to repair that flux capacitor first.
    – Doc Brown
    Commented Sep 28, 2017 at 19:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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