Reputation: 279
I have a Singleton class to save the state of an application's module. This class simply have a lot of class variables with setters and getters :
public class ModuleState{
private static ModuleState instance;
private A a;
private B b;
private C c;
..
..
..
..
private ModuleState (){}
public ModuleState getInstance(){
if(instance==null)
instance=new ModuleState();
return instance;
}
}
At a precise moment of the application lifecycle, i have the need to CLEAR the module's state. What i do now is to reset ALL the variables in ModuleState by a clearAll() method like this:
public void clearAll(){
a=null;
b=null;
c=null;
..
..
}
My question is the following : there is a cleaner method to do this reset? Possibly clearing the singleton instance itself, without resetting every class variable?
The problem with this approach is that i may have the need to add a new class variable to the ModuleState. In this case i must remember to add a line in the clearAll() method to reset the new variable.
Upvotes: 6
Views: 20852
Reputation: 13967
What about ...
public static volatile ModuleState instance = null;
public static void reset() {
instance = new ModuleState();
}
p.s.: as per discussion below: in a multithreaded environment it's very important to synchronize the access on the instance because the JVM is allowed to cache its value. You can use volatile
as shown above. Thanks to all!
Cheers!
Upvotes: 12
Reputation: 425418
Make an inner class to hold the fields, then replace that instance when you want to reset. The write to the field would make the change to all three fields essentially atomic.
public class ModuleState {
private static volatile ModuleState instance;
private static class Values {
A a;
B b;
C c;
}
private volatile Values values = new Values()(
private ModuleState (){}
public ModuleState getInstance(){
if (instance==null) {
synchronized (ModuleState.class) {
if (instance==null) {
instance = new ModuleState();
}
}
}
return instance;
}
public synchronized A getA() {
return values.a;
}
public synchronized void reset() {
values = new Values();
}
By the way, your null checking initialization code was not threadsafe. I fixed that too.
Note that to make this work, you must make the reference to values
volatile and synchronize all access to it, otherwise (due to the java memory model) other threads than the one that calls reset() may see the old reference.
Upvotes: 0
Reputation: 236
May be this can help you:
public class SingletonBean {
private static SingletonBean instance = new SingletonBean();
private static Object privateMutex = new Object();
private SingletonBean() {
//to prevent instantiation
}
public class ObjectsContainer {
private Object A;
private Object B;
private Object C;
public Object getA() {
return A;
}
public void setA(Object a) {
A = a;
}
public Object getB() {
return B;
}
public void setB(Object b) {
B = b;
}
public Object getC() {
return C;
}
public void setC(Object c) {
C = c;
}
}
private ObjectsContainer objectsContainer;
private void resetObjectsContainer() {
objectsContainer = new ObjectsContainer();
}
public static SingletonBean getInstance() {
return SingletonBean.instance;
}
public static void clearAll() {
synchronized (privateMutex) {
SingletonBean.getInstance().resetObjectsContainer();
}
}
public static ObjectsContainer getObjectsContainer() {
synchronized (privateMutex) {
return instance.objectsContainer;
}
}
}
public class SomeClass {
public void someMethod() {
SingletonBean.getObjectsContainer().getA();
}
}
Upvotes: 0
Reputation: 27356
You need to maintain the same object instance, in order to comply with the Singleton pattern, so your approach makes sense: altering the members.
However, if you wanted to clean it up a little bit, why not just have an internal list, like:
ArrayList<Object> members = new ArrayList<Object>();
// If it actually is Object, there's no need to paramaterize.
// If you want, you can actually make the members implement a common interface,
// and parameterize the ArrayList to that.
Another Option would be to have a HashMap
, that binds the key word to the member.
HashMap<String,Object> members = new HashMap<String,Object>();
// Again, same parameterization rules apply.
For an ArrayList
or a HashMap
, the clearAll
method might look like this:
public class ModuleState()
{
public void clearAll()
{
members.clear();
}
}
This method won't need to change.
Upvotes: 1
Reputation: 24202
no, this approach is perfectly acceptable. you are of course synchronizing access to these state objects in some way, right? otherwise you risk someone seeing a half-cleared config object.
another thing you could do to future-proof yourself against any extra state added in the future is store all of your state in a HashMap, for example, instead of individual fields. this way, clear()ing the hashmap ensures that all state is wiped and adding any extra state in the future becomes safer
Upvotes: 2