Is this a correct Singleton implementation?

Question

Possible Duplicate:
What is an efficient way to implement a singleton pattern in Java?

I am new to design pattern and in learning phase. Recently I learned about Singleton pattern and I looked around our company's code where I work. I have found the following code snippet which looks incorrect according to the Singleton rules i.e we should not synchronize a method but sunchronize a block. And thread should always check whether the "ïnstance" variavle's value is null or not before entering the block. Is my understanding is correct or there is some logic to code in following manner?

public class CustomLogger {

private static CustomLogger instance;

private CustomLogger(){
    some code here....
}

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

FYI: [What is an efficient way to implement a singleton pattern in Java?](http://stackoverflow.com/questions/70689/what-is-an-efficient-way-to-implement-a-singleton-pattern-in-java/71399#71399) — Alvin Wong, Jan 29 '13 at 05:58

score 1 · Answer 1 · answered Jan 29 '13 at 05:58

You approach is correct since your getInstance() method is synchronized.But the synchronization is required only for the first invocation of getInstance().That is, when we are creating the singleton object.All other cases we need already created instance and that doesn't require synchronized access.

Another approach is synchronizing the singleton creation part alone. but that approach(double locking) have many problems. Refer this : http://www.ibm.com/developerworks/java/library/j-dcl/index.html

From Java 5, onwards the best approach for creating singleton would be using Enums.

score 1 · Answer 2 · answered Jan 29 '13 at 06:01

If you must use singletons, just construct them eagerly without all the funny business with synchronization. That is:

public class CustomLogger {
  private static CustomLogger instance = new CustomLogger();

  public static CustomLogger instance() { return instance;
}

there rarely is any benefit from lazy initialization of the singleton instance within class -- instance will be constructed when class in question is loaded for active use, which is typically when instance() is called, which itself should be when lazy instantiation would happen anyway.

I suspect reason for code you showed was due to old C++ programmers being wary of loading order; something that is not a problem in Java (loading order is well specified in JLS).

score 0 · Answer 3 · answered Jan 29 '13 at 05:58

we should not synchronize a method but sunchronize a block.

Wrong. Both are fine, but sunchronize a block is a better way to do.

check whether the "ïnstance" variables's value is null or not before entering the block

Thats what this line is doing

if (instance == null)

Is this a correct Singleton implementation?

3 Answers3