Sua classe está mais ou menos. Mas o primeiro grande problema que eu vejo é o construtor ser público. Com isso, basta alguém invocá-lo e o seu singleton não mais será um singleton.
Há outro grande problema também: Sincronização. Basta duas Threads
invocarem o getInstance()
ou o setContext(Context)
ao mesmo tempo e com um pouco de azar você pode acabar com duas instâncias do singleton, ou então com dois contextos distintos.
Além disso, há uma questão importante: Porque instanciar a classe Global
de forma lazy? Ela não tem nenhum recurso pesado a ser instanciado no construtor. Com isso a sua classe fica assim:
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;
}
}
E assim já está legal, principalmente se você pretender adicionar mais coisas no seu singleton.
Algo que me preocupa um pouco é esse setContext()
. O ideal é que o seu singleton seja imutável, e uma vez devidamente configurado, nunca mais seria mexido. Mas antes disso, vem a pergunta: Como você faz para obter o Context
? Há vários contextos no android, qual contexto exatamente você está armazenando no seu singleton e porque você precisa fazer isso?
Além disso, dependendo do caso (não é em todo e qualquer singleton), você pode deixar os seus métodos estáticos, eliminando a necessidade de ter o 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;
}
}
E então você usaria apenas Global.getContext();
ao invés de Global.getInstance().getContext();
. Além disso, você nunca veria uma instância de Global
fora da própria classe Global
(o que é ótimo, uma vez que isso provavelmente não faria sentido).
Voltando na questão de multithreading, para evitar de ter os métodos synchronized
você pode fazer a variável de instância context
ser 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;
}
}
Uma outra possibilidade é usar AtomicReference<Context>
como o campo da classe:
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);
}
}