Constructor newInstance generates local instance only

Question

Looks I miss something in my tests (Robolectrics|Powermockito).

I have following class with Singleton:

public class Core{

   private static Core instance = new Core();

   public static Core getInstance() {
        return instance;
    }   

   public static void destroy(){
    instance = null;
   } 
}

In my tests I kill the instance with Core.destroy() and therefore Core.getInstance() returns null.

So every test I want to re-generate instance again. I do the following:

Constructor<Core> constructor = Core.class.getDeclaredConstructor();
                constructor.setAccessible(true);
                Core newCore = constructor.newInstance();

So now newCore is initialized but Core.getInstance() still returns null.

How to initialize properly Core -> instance?

Answer 1

public class Core {

    private static class SingletonHolder {
        private static AtomicReference<Core> instance = new AtomicReference(new Core());
    }

    public static Core getInstance() {
        return SingletonHolder.instance.get();
    }   

    public static void destroy() {
        SingletonHolder.instance.set(null);
    } 

    public static void reset() {
        SingletonHolder.instance.compareAndSet(null, new Core());
    } 
}

Using an extra "superfluous" inner class is done for concurrent initialisation, ensuring that the static field is initialized once.

Changing the instance (destroy, my reset) needs some kind of synchronisation for objects. Instead of the more costly synchronize one can use an AtomicReference.

compareAndSet does not set the instance with a new value, if there is already an old value.

It is also worth having

Optional<Core> getInstance() { ... }

So the usage is safe-guarded.

Core.getInstance().ifPresent(core -> { ... core ... });

Answer 2

You should make the constructor private so that code using you singleton class cannot create an instance using it and they should only get an instance using the getInstance() method.

Also the lifetime of a singleton object is typically tied to the JVM, as there should be a single instance of a singleton class per JVM. So if you can destroy and re-create the instance it is not a true Singleton IMO, so I assume you only want to re-create the instance for testing.

To re-create the singleton from your test classes after calling the destroy() method you can get the Field of the class having the instance of your class. Using that Field you can set it to the new instance you created:

public static void main(String[] args) throws Exception {
        System.out.println(Core.getInstance()); //gets instance
        Core.destroy();
        System.out.println(Core.getInstance()); // null
        reinitializeInstance(Core.class);
        System.out.println(Core.getInstance()); //gets instance
 }

public static void reinitializeInstance(Class<Core> clazz) {
    try {
        Constructor<Core> constructor = clazz.getDeclaredConstructor();
        constructor.setAccessible(true);
        Core newCore = constructor.newInstance();

        Field field = Core.class.getDeclaredField("instance"); //gets the instance field
        field.setAccessible(true);
        field.set(newCore, newCore);

    } catch (Exception e) {
        e.printStackTrace();
    }
}

And your Singleton class:

class Core {

    private static Core instance = new Core();

    // To prevent reflection from creating a new instance without destroying the first one
    private Core() {
       if(instance != null){
           throw new IllegalStateException("Instance already exists!");
       }
    }

    public static Core getInstance() {
        return instance;
    }

    public static void destroy() {
        instance = null;
    }

}

Answer 3

There is an important point that I often try to explain to people when talking about singletons:

There's a difference between a singleton and something that you will only create 1 instance of. And, often, when you think you want a singleton, actually you just want something that you will only create 1 instance of.

The difference between these two things is perhaps not apparent at first, but important to realize, especially when you find yourself in a position where you need to purge the internal state of a singleton between tests.

• If you have a singleton - a true singleton - there is, by definition, one instance that can exist in the JVM. If this has mutable state, this is problematic, because it means that you have to care about that state. In tests, you have to clear state between runs to remove any effects owing to the ordering of test execution; and you have to run your tests serially.

• If you use dependency injection (as in the concept, not any particular framework like Guice, Dagger, Spring etc), it doesn't matter to classes using the instance where that instance came from: you, as a client of the class, get control over its life cycle. So, whereas your production code uses the same instance in all places, your testing code can use separate instances - thus they are decoupled - and often you don't even have to worry about cleaning up state at all, because your next test case can simply create a new instance of the class.

So, instead of code using your Core class like so:

class MyClass {
  void foo() {
    Core core = Core.getInstance();
    // ... do stuff with the Core instance.
  }
}

you can write it instead like:

class MyClass {
  private final Core core;

  MyClass(Core core) { this.core = core; }

  void foo() {
    // ... do stuff with the Core instance.
  }
}

and you have broken the static binding between MyClass and Core. You can instantiate MyClass in tests with separate instances of Core:

MyClass myClass = new MyClass(new Core());
// Assert something...

or, if multiple instances need to interact with the same instance of Core:

Core core = new Core();
MyClass myClass = new MyClass(core);
MyOtherClass myOtherClass = new MyOtherClass(core);
// Assert something...

Answer 4

How about this pattern instead?

public class Core{

   private static Core instance;

   public static Core getInstance() {
        if(instance == null) instance = new Core();
        return instance;
    }   

   public static void destroy(){
    instance = null;
   } 
}

And if you only want to destory in tests, you can remove "public" from your destroy() method

Answer 5

First of all you're making Core constructor accessible, but it's public by default already.

Second, when your calling the constructor, it just creates a new instance of Core, which does nothing to instance, because constructor, created by default is empty and because constructor is not the place to initialize Singleton.

If you want to refresh singleton instance you should have a dedicated method for that.