Your class is more or less. But the first big problem I see is the builder being public. With this, just someone summon you and your Singleton will no longer be a Singleton.
There is another big problem too: Synchronization. Just two Threads
invoke the getInstance()
or the setContext(Context)
at the same time and with a bit of bad luck you can end up with two instances of Singleton, or with two different contexts.
In addition, there is an important question: Why to instantiate the class Global
so Lazy? She has no heavy resource to be instantiated in the constructor. With that her class gets like this:
public class Global {
private static final Global instance = new Global();
private Context context;
private Global() {
}
public static Global getInstance() {
return instance;
}
public synchronized Context getContext() {
return context;
}
public synchronized void setContext(Context context) {
this.context = context;
}
}
And so it’s already cool, especially if you intend to add more things in your Singleton.
One thing that worries me a little bit is this setContext()
. The ideal is that your Singleton is immutable, and once properly configured, it would never be touched again. But before that comes the question: How do you get the Context
? There are several contexts on android, which context exactly you are storing in your Singleton and why you need to do this?
Moreover, depending on the case (not at all and any Singleton), you can leave your methods static, eliminating the need to have the getInstance()
:
public class Global {
private static final Global instance = new Global();
private Context context;
private Global() {
}
public static synchronized Context getContext() {
return instance.context;
}
public static synchronized void setContext(Context context) {
instance.context = context;
}
}
And then you’d just use Global.getContext();
instead of Global.getInstance().getContext();
. Also, you would never see an instance of Global
out of class Global
(which is great, since that probably wouldn’t make sense).
Returning to the question of multithreading, to avoid having the methods synchronized
you can make the instance variable context
was volatile
:
public class Global {
private static final Global instance = new Global();
private volatile Context context;
private Global() {
}
public static Context getContext() {
return instance.context;
}
public static void setContext(Context context) {
instance.context = context;
}
}
Another possibility is to use AtomicReference<Context>
as the field of class:
public class Global {
private static final Global instance = new Global();
private final AtomicReference<Context> contextRef = new AtomicReference<>();
private Global() {
}
public static Context getContext() {
return instance.contextRef.get();
}
public static void setContext(Context context) {
instance.contextRef.set(context);
}
}
I agree with Rio. And I add that it is never good to maintain an instance of
Context
because it’s a heavy object, especially if it’s aActivity
. Often the calls to methods depend on the life cycle, which is different for each one.– Wakim
Thanks Wakim! if you see the link at the end of the reply.. there is a table that shows the capabilities of each type of context
– dariodm
Thanks, I didn’t know much about the context, now I have a better idea of how to work with him.
– Renan Lazarotto