32

What is the best/correct way to create a singleton class in java?

One of the implementation I found is using a private constructor and a getInstance() method.

package singleton;

public class Singleton {

    private static Singleton me;

    private Singleton() {
    }

    public static Singleton getInstance() {
        if (me == null) {
            me = new Singleton();
        }

        return me;
    }
}

But is implementation fails in the following test case

package singleton;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;

public class Test {

    /**
     * @param args
     * @throws NoSuchMethodException
     * @throws SecurityException
     * @throws InvocationTargetException
     * @throws IllegalAccessException
     * @throws InstantiationException
     * @throws IllegalArgumentException
     */
    public static void main(String[] args) throws SecurityException,
            NoSuchMethodException, IllegalArgumentException,
            InstantiationException, IllegalAccessException,
            InvocationTargetException {
        Singleton singleton1 = Singleton.getInstance();
        System.out.println(singleton1);

        Singleton singleton2 = Singleton.getInstance();
        System.out.println(singleton2);

        Constructor<Singleton> c = Singleton.class
                .getDeclaredConstructor((Class<?>[]) null);
        c.setAccessible(true);
        System.out.println(c);

        Singleton singleton3 = c.newInstance((Object[]) null);
        System.out.println(singleton3);

        if(singleton1 == singleton2){
            System.out.println("Variable 1 and 2 referes same instance");
        }else{
            System.out.println("Variable 1 and 2 referes different instances");
        }
        if(singleton1 == singleton3){
            System.out.println("Variable 1 and 3 referes same instance");
        }else{
            System.out.println("Variable 1 and 3 referes different instances");
        }
    }

}

How to resolve this?

Thank you

Arun P Johny
  • 384,651
  • 66
  • 527
  • 531
  • *"But is implementation fails in the following test case"* -- What exactly do you mean by "fails?" – cdhowie Dec 06 '10 at 03:24
  • It allows to creates multiple instance throw reflection – Arun P Johny Dec 06 '10 at 03:25
  • 7
    The best way, in 99.99% of cases, based on my years of professional experience, is to **not do it**. What do you think you need a Singleton for, really? – Karl Knechtel Dec 06 '10 at 03:27
  • I don't see why it fails, but you may want to put a print statement in the if statement before creating a new singleton, and see if you only get one. If so then your test is incorrect. – James Black Dec 06 '10 at 03:27
  • 4
    Firstly, singletons are evil. Secondly, singletons are global variables, Thirdly, don't use singletons. Also, you cannot stop reflection from being able to bad things to your class. DO NOT TRY! – Winston Ewert Dec 06 '10 at 03:29
  • 1
    I think I'll take your advice, if somebody uses reflection to mess up.. it is their problem.. not mine. – Arun P Johny Dec 06 '10 at 03:37
  • @Winston Ewert, Can you explain the point about global variables? – Arun P Johny Dec 06 '10 at 03:39
  • One of the only times I use the singleton pattern is when implementing such things as [IEqualityComparer](http://msdn.microsoft.com/en-us/library/ms132151.aspx) or other such classes where there is no benefit whatsoever to allowing multiple instances. But I *still* make the constructors public, just in case someone needs a unique instance for whatever reason... – cdhowie Dec 06 '10 at 03:57
  • @Arun, why do you want to use a singleton? Because you want to have a global object which you can access from anywhere in your application. Hence, your singleton is a global variable. (Unless your singleton never changes then its a global constant.) Global variables have various issues such as implicit coupling, testing difficulty, etc. Very few of these issues are addressed by using a singleton. – Winston Ewert Dec 06 '10 at 03:59
  • @cdhowie, I see why you don't need unique instances, but I'm not sure why you feel the need to have only one instance. Is it just for performance reasons? – Winston Ewert Dec 06 '10 at 04:02
  • @Winston: Less GC churn is always better in my opinion. It's really not much harder to write `SomeEqualityComparer.Instance` than `new SomeEqualityComparer()`. In the .NET framework, MS uses this pattern a lot too. See, for example, [EventArgs.Empty](http://msdn.microsoft.com/en-us/library/system.eventargs.empty.aspx) and [Stream.Null](http://msdn.microsoft.com/en-us/library/system.io.stream.null.aspx). The latter is interesting in that the actual object returned is of a Stream subclass that is not public. – cdhowie Dec 06 '10 at 04:07
  • I've a properties file containing some keys value pairs, which is need across the application, that is why I was thinking about a singleton class. This class will load the properties from a file and keep it and you can use it from anywhere in the application. – Arun P Johny Dec 06 '10 at 04:20
  • @cdhowie, I see. My only concern there would be that I would be slightly confused upon seeing it for the first time. Its not something I would expected to be made a singleton. Ideally, you'd really like the sort of optimization you are doing there to be automatic, but alas, it is not. – Winston Ewert Dec 06 '10 at 04:27
  • 4
    @Arun, I'd create the object once and pass it to the constructor of all the objects that need the settings. – Winston Ewert Dec 06 '10 at 04:31
  • @Winston That is a better idea, but I thought it will be easy to do it with a singleton class since I don't have to create keep on passing the object every time. – Arun P Johny Dec 06 '10 at 04:38
  • 1
    @Arun: The road to programming Hell is paved with "it would be easy to do with..." – JUST MY correct OPINION Dec 06 '10 at 07:04
  • @Arun, it is easier to use a singleton. But you gain greater flexibility by not using one. For example, it is far easier to write unit tests when you have no singletons. It also should be easier to adapt the program should the singleton cease to be global at some point. (As an aside, no rule is absolute. There are cases where singletons are the best solution. IMHO, they are rare. Rare enough that I've not written a singleton in years (excepting one sillly case where I was forced to do so)) – Winston Ewert Dec 06 '10 at 14:21
  • @Arun, if you find yourself passing this settings objects all over the program, its a sign that your design could be better in other ways. Ideally, you construct all your important objects together which will make it easy to pass the settings objects to the constructors for all of the other objects. – Winston Ewert Dec 06 '10 at 14:23
  • 3
    guys, the question asked is to resolve the issue, not to go Dr. House about if singeltons are good or bad. – tony9099 Nov 04 '13 at 21:03
  • http://stackoverflow.com/questions/20945049/is-a-java-string-really-immutable#comment-31456276 – cHao Jan 19 '14 at 16:04

7 Answers7

17

As per the comment on your question:

I've a properties file containing some keys value pairs, which is need across the application, that is why I was thinking about a singleton class. This class will load the properties from a file and keep it and you can use it from anywhere in the application

Don't use a singleton. You apparently don't need one-time lazy initialization (that's where a singleton is all about). You want one-time direct initialization. Just make it static and load it in a static initializer.

E.g.

public class Config {

    private static final Properties PROPERTIES = new Properties();

    static {
        try {
            PROPERTIES.load(Thread.currentThread().getContextClassLoader().getResourceAsStream("config.properties"));
        } catch (IOException e) {
            throw new ExceptionInInitializerError("Loading config file failed.", e);
        }
    }

    public static String getProperty(String key) {
        return PROPERTIES.getProperty(key);
    }

    // ...
}
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • Thanks for your suggestion. I would try to implement this. – Arun P Johny Dec 06 '10 at 04:42
  • 2
    If you're new to static initializers, you may find [this answer](http://stackoverflow.com/questions/1893274/doubt-in-java-basics/1893321#1893321) useful as well. – BalusC Dec 06 '10 at 04:48
  • This is still a singleton. It's just a singleton that is not lazy loaded. WTH is going on here... ?? – Rob Jan 10 '14 at 00:02
  • @Rob: This is not a capital-S Singleton, though. (1) The single instance is of a different type, and (2) it's never directly exposed -- only its behavior is. It's still pretty ugly IMO, but it at least doesn't pretend to be a discrete object when it's not. – cHao Jan 19 '14 at 22:49
  • @nikel: because it's not optional. – BalusC Dec 12 '14 at 16:24
  • Well, its rare but there can be some flows in an application which might not need the properties to be loaded. – nikel Dec 12 '14 at 19:32
  • @nikel: I'd put question marks on the application itself. – BalusC Dec 12 '14 at 21:54
4

I will implement singleton in below way.

From Singleton_pattern described by wikiepdia by using Initialization-on-demand holder idiom

This solution is thread-safe without requiring special language constructs (i.e. volatile or synchronized

public final class  LazySingleton {
    private LazySingleton() {}
    public static LazySingleton getInstance() {
        return LazyHolder.INSTANCE;
    }
    private static class LazyHolder {
        private static final LazySingleton INSTANCE = new LazySingleton();
    }
    private Object readResolve()  {
        return LazyHolder.INSTANCE;
    }
}
Ravindra babu
  • 37,698
  • 11
  • 250
  • 211
4

If you are using reflection to pierce encapsulation, you should not be surprised when behavior of your class is altered in incorrect ways. Private members are supposed to be private to the class. By using reflection to access them you are intentionally breaking the behavior of the class, and the resultant "duplicate singleton" is expected.

In short: Don't do that.

Also, you might consider creating the singleton instance in a static constructor. Static constructors are synchronized and will only run once. Your current class contains a race condition -- if two separate threads call getInstance() when it has not been previously called, there is a possibility that two instances will be created, one of them being exclusive to one of the threads, and the other becoming the instance that future getInstance() calls will return.

cdhowie
  • 158,093
  • 24
  • 286
  • 300
0

Best way to create Singleton Class in java is using Enums.

Example as below :

import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method; 

enum SingleInstance{
    INSTANCE;

    private SingleInstance() {
        System.out.println("constructor");
    }   
}

public class EnumSingletonDemo {

    public static void main (String args[]) throws FileNotFoundException, IOException, ClassNotFoundException, NoSuchMethodException, SecurityException, InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException
    {
        SingleInstance s=SingleInstance.INSTANCE;
        SingleInstance s1=SingleInstance.INSTANCE;

        System.out.println(s.hashCode() + " "+s1.hashCode());//prints same hashcode indicates only one instance created

    //------- Serialization -------
    ObjectOutputStream oos=new ObjectOutputStream(new FileOutputStream("sample.ser"));
    oos.writeObject(s);
    oos.close();

    //------- De-Serialization -------
    ObjectInputStream ois=new ObjectInputStream(new FileInputStream("sample.ser"));
    SingleInstance s2=(SingleInstance) ois.readObject();

    System.out.println("Serialization :: "+s.hashCode()+" "+s2.hashCode());// prints same hashcodes because JVM handles serialization in case of enum(we dont need to override readResolve() method)

   //-----Accessing private enum constructor using Reflection-----

    Class c=Class.forName("SingleInstance");

    Constructor co=c.getDeclaredConstructor();//throws NoSuchMethodException
    co.setAccessible(true);
    SingleInstance newInst=(SingleInstance) co.newInstance();           

}
}

NoSuchMethodException is thrown because we can't create another instance of enum 'SingleInstance' through its private constructor using Reflection.

In Case of Serialization enum implements serializable interface by default.

kalyani chaudhari
  • 7,515
  • 3
  • 24
  • 21
-1

I think you can check whether an instance already exists in the constructor and if exists throw an exception

if(me != null){
    throw new InstanceAlreadyExistsException();
}
Ram
  • 409
  • 1
  • 4
  • 7
  • There is no need to do this in a private member, since private members should not be accessed outside the class. If someone wants to use reflection to access private members, the resultant behavior is their problem. – cdhowie Dec 06 '10 at 03:30
-1
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;

public class DBConnection {


    private static DBConnection dbConnection;
    private Connection connection;

    private DBConnection() throws ClassNotFoundException, SQLException {
        Class.forName("com.mysql.jdbc.Driver");
        connection = DriverManager.getConnection(/*crate connection*/);
    }

    public Connection getConnection(){
        return connection;
    }
    public static DBConnection getInstance() throws SQLException, ClassNotFoundException {
        return (null==dbConnection) ? (dbConnection = new DBConnection()) : dbConnection;
    }
}

 
  • Can you please add an explanation to your answer? Why does using the code you provided work and what is the problem? – MkMan Sep 20 '20 at 22:19
-2

just follow the singleton pattern class diagram,

SingletonClass - singletonObject: SingletonClass - SingletonClass() + getObject(): SingletonClass

Key point,

  • private your constructor
  • the instance of your class should be inside the class
  • provide the function to return your instance

Some code,

public class SingletonClass {
    private static boolean hasObject = false;
    private static SingletonClass singletonObject = null;

    public static SingletonClass getObject() {
        if (hasObject) {
            return singletonObject;
        } else {
            hasObject = true;
            singletonObject = new SingletonClass();
            return singletonObject;
        }
    }

    private SingletonClass() {
        // Initialize your object.
    }
}
Mr. Polywhirl
  • 42,981
  • 12
  • 84
  • 132
Chu Xiwen
  • 41
  • 3
  • This code is inherently not thread-safe. If two threads are in `getObject()` at exactly the same time, two instances can be created. – cHao Jan 19 '14 at 22:43