views:

377

answers:

6

Hi Guys,

Whats the best way to design a singleton class that could throw an exception?

Here I have a Singleton (using Bill Pugh's method, documented in Wiki for Singleton).

    private static class SingletonObjectFactoryHolder{
    //1  
        private static final ObjectFactory INSTANCE = new ObjectFactory();
    }

    private ObjectFactory() throws Exception{
    //2
            //create the factory
    }


    public static ObjectFactory getInstance(){
    //3
        return SingletonObjectFactoryHolder.INSTANCE;
    }

If an exception is thrown at 2, I would like to propagate it to the caller. However, I can't throw an exception from line 1.

So, is my only option to return a null object if the singleton object wasn't created correctly?

Thanks

P.S I do realize that this Singleton can break if its loaded via different classloaders or if loaded reflexively, but it's good enough for my purpose.

//UPDATE

I am curious, can I not rearrange my design as below to throw exceptions?

Also, I don't need any synchronization (the classloader guarantees that the static inner class will only loaded once and only when getInstance() is called). Thus, thread-safe and lazily-instantiated?

 private static class SingletonObjectFactoryHolder{
        //1  
           public static ObjectFactory getInstance() throws Exception{
         return new ObjectFactory();
           }
 }

 private ObjectFactory() throws Exception{
        //2
        //create the factory
 }


 public static ObjectFactory getInstance(){
        //3
    return SingletonObjectFactoryHolder.getInstance();
 }

Thanks again.

A: 

Just don't throw exception from object's constructor. You can provide init () method and throw your exception from there if it's necessary.

Roman
A: 

Make INSTANCE not final and create it lazy in getInstance() if null:

public static ObjectFactory getInstance() throws Exception {
  if (SingletonObjectFactoryHolder.INSTANCE == null) {
    synchronized (ObjectFactory.class) {
      if (SingletonObjectFactoryHolder.INSTANCE == null) {
        SingletonObjectFactoryHolder.INSTANCE = new ObjectFactory();
      }
    }
  }
  return SingletonObjectFactoryHolder.INSTANCE;
}

ok, more simple and safe:

public static synchronized ObjectFactory getInstance() throws Exception {
  if (SingletonObjectFactoryHolder.INSTANCE == null) {
    SingletonObjectFactoryHolder.INSTANCE = new ObjectFactory();
  }
  return SingletonObjectFactoryHolder.INSTANCE;
}

but the best of all: do not throw an exception from constructor...

Arne Burmeister
If you do it lazily make sure you do your synchronization correctly if your using it in a multithreaded context. Depending on the degree the singleton is accessed via getInstance you may want to consider using a reader-writer lock rather than a simple monitor.
Robert Davis
Double checked locking is broken in many ways. If you can guarantee Java >= 5, it is safe IF AND ONLY IF YOU USE A VOLATILE VARIABLE.http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.htmlSorry to be a hater, but using double checked locking is a great way to get really strange concurrency bugs, so it really should be avoided...
Steven Schlansker
Right, INSTANCE should become volatile, or simply make getInstance() synchronized. Much better would be the normal singleton without exception on constructor...
Arne Burmeister
A: 

You could check for null INSTANCE inside of getInstance, and lazily initialize it. But typically its best if constructors don't throw, if you can make the constructor safer that would probably be best.

Zac
A: 

I agree with Arne Burmeister, the code for that would look like:

private static class SingletonObjectFactoryHolder
{
    private static ObjectFactory INSTANCE;


    private ObjectFactory() 
    {

    }

    public static String getInstance() throws Exception
    {
      return (INSTANCE == null) ? (INSTANCE = new ObjectFactory()) : INSTANCE; 
     // A ternary operator might not be the best fit if you need to throw an exception, but I think it looks nicer than the if(INSTANCE==null){} else{} for lazy instantiation.
    }

}
instanceofTom
A: 

Actually, the key is to make sure getInstance is synchronized, because it must be thread safe. And, you no longer have SingletonObjectFactoryHolder.

public class ObjectFactory {
    private static final ObjectFactory INSTANCE;

private ObjectFactory() throws Exception{
}

public synchronized static ObjectFactory getInstance(){
   if( INSTANCE == null )
   INSTANCE = new ObjectFactory();
   return INSTANCE
}
}
harschware
+4  A: 

Use a static initializer and rethrow the Exception as ExceptionInInitializerError. Click the link to read the Javadoc, you'll see that it suits exactly for this particular functional requirement: handling exceptions during static initialization. A singleton is in fact nothing less or more than a statically initialized global object.

private static class SingletonObjectFactoryHolder{
    private static final ObjectFactory INSTANCE;
    static {
        try {
            INSTANCE = new ObjectFactory();
        } catch (Exception e) {
            throw new ExceptionInInitializerError(e);
        }
    }
}

No need for double checked locking idiom which is considered an anti-pattern and in some circumstances even unsafe.

BalusC
(+1) Thanks for teaching me about EIIE
harschware