Singleton - Best way to instantiate class

Question

I was looking at Telegrams's messenger source code and I noticed that their singleton classes all use local variables on their getInstance methods, just like below. For example, on their Android GitHub repo, on class NotificationsController.java they have the following:

private static volatile NotificationsController Instance = null;
public static NotificationsController getInstance() {
    NotificationsController localInstance = Instance;
    if (localInstance == null) {
        synchronized (MessagesController.class) {
            localInstance = Instance;
            if (localInstance == null) {
                Instance = localInstance = new NotificationsController();
            }
        }
    }
    return localInstance;
}

I'm not entirely sure what is the purpose of the local var "localInstance" there. Can anyone explain exactly what is the purpose of the "localInstance" var? Could not the same be achieved without it, just like in the code below?

private static volatile NotificationsController Instance = null;
public static NotificationsController getInstance() {
    if (Instance == null) {
        synchronized (MessagesController.class) {
            if (Instance == null) {
                Instance = new NotificationsController();
            }
        }
    }
    return Instance;
}

In your version, you are also missing the second `if` check inside the `synchronized` block. That means the constructor may be called twice. — Thilo, Mar 08 '16 at 01:29
Just looks like bad code to me. For starters Singleton pattern above is an anti-pattern http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons. Second of all, they're synchronizing on the class object of a different class which could cause deadlock if any other piece of code decides to take a lock on that object, third of all, the point you raise. That GitHub only has one primary contributor. I would be hesitant to use that library. — Samuel, Mar 08 '16 at 01:31

score 1 · Accepted Answer · edited May 23 '17 at 12:31

This is done for performance reasons.

Consider the most common scenario where the variable has been initialized. The code as written will read the volatile variable once and return the value. Your version would read it twice. Since volatile reads carry a slight performance cost, using the local variable can be faster.

Because in your case the lazily initialized variable is a static, it is preferable to use the holder class idiom. See this answer for an example.

score 0 · Answer 2 · answered Mar 08 '16 at 02:21

0

Sure this article about "Safe publication in Java" http://shipilev.net/blog/2014/safe-public-construction/ will help you to understand how it works for singletones

answered Mar 08 '16 at 02:21

ivanenok

594
5
9

Singleton - Best way to instantiate class

2 Answers2