views:

2418

answers:

10

I was reading the Singleton article on Wikipedia and I came across this example:

public class Singleton {
    // Private constructor prevents instantiation from other classes
    private Singleton() {}

    /**
     * SingletonHolder is loaded on the first execution of Singleton.getInstance() 
     * or the first access to SingletonHolder.INSTANCE, not before.
     */
    private static class SingletonHolder { 
     private static final Singleton INSTANCE = new Singleton();
    }

    public static Singleton getInstance() {
     return SingletonHolder.INSTANCE;
    }
}

While I really like the way this Singleton behaves, I can't see how to adapt it to incorporate arguments to the constructor. What is the preferred way to do this in Java? Would I have to do something like this?

public class Singleton
{
    private static Singleton singleton = null;  
    private final int x;

    private Singleton(int x) {
     this.x = x;
    }

    public synchronized static Singleton getInstance(int x) {
     if(singleton == null) singleton = new Singleton(x);
     return singleton;
    }
}

Thanks!


Edit: I think I have started a storm of controversy with my desire to use Singleton. Let me explain my motivation and hopefully someone can suggest a better idea. I am using a grid computing framework to execute tasks in parallel. In general, I have something like this:

// AbstractTask implements Serializable
public class Task extends AbstractTask
{
    private final ReferenceToReallyBigObject object;

    public Task(ReferenceToReallyBigObject object)
    {
     this.object = object;
    }

    public void run()
    {
     // Do some stuff with the object (which is immutable).
    }
}

What happens is that even though I merely pass a reference to my data to all of the tasks, when the tasks are serialized, the data gets copied over and over. What I want to do is share the object among all of the tasks. Naturally, I might modify the class like so:

// AbstractTask implements Serializable
public class Task extends AbstractTask
{
    private static ReferenceToReallyBigObject object = null;

    private final String filePath;

    public Task(String filePath)
    {
     this.filePath = filePath;
    }

    public void run()
    {
     synchronized(this)
     {
      if(object == null)
      {
       ObjectReader reader = new ObjectReader(filePath);
       object = reader.read();
      }
     }

     // Do some stuff with the object (which is immutable).
    }
}

As you can see, even here I have the issue that passing a different file path means nothing after the first one is passed. This is why I like the idea for a store which was posted in the answers. Anyhow, rather than including the logic for loading the file in the run method, I wanted to abstract this logic into a Singleton class. I will not provide yet another example, but I hope you get the idea. Please let me hear your ideas for a more elegant way to accomplish what I am trying to do. Thank you again!

+2  A: 

Singletons are generally considered to be anti-patterns and shouldn't be used. They do not make code easy to test.

A singleton with an argument makes no sense anyway - what would happen if you wrote:

Singleton s = SingletonHolder.getInstance(1);
Singleton t = SingletonHolder.getInstance(2); //should probably throw IllegalStateException

Your singleton is also not thread-safe as multiple threads can make simultaneous calls to getInstance resulting in more than one instance being created (possibly with different values of x).

oxbow_lakes
That's fairly debatable.
AlbertoPL
Yes it is debatable; hence my use of the word "generally". I think it is fair to say that they are generally considered a bad idea
oxbow_lakes
It is debatable - some people claim that what are called "anti-patterns" fit the definition of patterns, it's just that they are bad patterns.
Tom Hawtin - tackline
I understand that they are bad. I am doing distributed computing and need to share an object among multiple tasks. Rather than deterministically initializing a static variable, I would like to abstract the logic into a Singleton. I imagine I could make getInstance synchronized. Would this work? What I need to do is load a file once for many tasks, and only after the first task has been sent. (I don't want my data serialized.) I thought I would make my AbstractFileReader an argument to the getInstance method in order to make the Singleton more flexible. I value your input.
I think you may misunderstand what "distributed" means? There are other ways of achieving what you want: have you considered dependency injection? Or JNDI?
oxbow_lakes
If you want to instantiate a number of file readers and reuse them, why not just use a Map<String,AbstractFileReader>, keyed on filename? You instantiate them as you need them and store them in the map (with proper synchronisation or using the java.util.concurrent maps).
George
+1  A: 

Use getters and setters to set the variable and make the default constructor private. Then use:

Singleton.getInstance().setX(value);
AlbertoPL
Don't get why this was down-voted.. It's a valid answer tbh. :/
Zack
Neither do I...?
AlbertoPL
A: 

The reason you can't make sense of how to accomplish what you're trying to do is probably that what you're trying to do doesn't really make sense. You want to call getInstance(x) with different arguments, but always return the same object? What behavior is it you want when you call getInstance(2) and then getInstance(5)?

If you want the same object but for its internal value to be different, which is the only way it's still a singleton, then you don't need to care about the constructor at all; you just set the value in getInstance() on the object's way out. Of course, you understand that all your other references to the singleton now have a different internal value.

If you want getInstance(2) and getInstance(5) to return different objects, on the other hand, you're not using the Singleton pattern, you're using the Factory pattern.

chaos
+3  A: 

I think you need something like a factory? To have objects with various parameters instantiated and reused? It could be implemented by using a synchronized HashMap or ConcurrentHashMap map a parameter (an Integer for an example) to your 'singleton' parameterizable class.

Although you might get to the point where you should use regular, non-singleton classes instead (for example needing 10.000 differently parametered singleton).

Here is an example for such store:

public final class UsefulObjFactory {
    private static Map<Integer, UsefulObj> store =
        new HashMap<Integer, UsefulObj>();
    public static final UsefulObj {
        private UsefulObj(int parameter) {
            // init
        }
        public void someUsefulMethod() {
            // some useful operation
        }
    }
    public static UsefulObj get(int parameter) {
        synchronized (store) {
            UsefulObj result = store.get(parameter);
            if (result == null) {
                result = new UsefulObj(parameter);
                store.put(parameter, result);
            }
            return result;
        }
    }
}

To push it even further, the Java enums can be also considered (or used as) parametered singletons, allowing although only a fixed number static variants.

However if you need a distributed1 solution, consider some lateral caching solution. For example: EHCache, Terracotta, etc.

1 in the sense of spanning multiple VMs on probably multiple computers.

kd304
This would work, excellent work. Very clever. :)
Zack
Yes, this is exactly what I need. Thank you very much! I agree that the way I was handling arguments in my example didn't make much sense, but I didn't think of this. See my explanation in the comments of oxbow_lakes' answer.
This is **NOT** a singleton; you now have more than one of them. LOL
oxbow_lakes
@Scott: I'd suggest something like what Yuval suggested below. It makes a little more sense and you have a 'true' singleton. [edit]
Zack
I hope no-one minds me editing the names in the code; I can imagine this is really confusing for newbies. Rollback if you disagree
oxbow_lakes
Yes, we could call them Multitron and still achieve the same goal the OP wanted in the first place IMHO.
kd304
Please no-one suggest that anyone should be using terracotta unless *they really know what they are doing**!
oxbow_lakes
@oxbow_lakes: maybe its time to ask for integrated refactoring support of the editor :). And for the Terracotta - just wanted to list some options based on OP's comment in your answer.
kd304
I give up. No cognitive energy left for the modified question.
kd304
@Zack: Yuval's solution wont work in my case, because it fails to abstract the logic I was initially trying to remove from the run method. If I used Yuval's Singleton, I would have something like this:synchronized(this){ if(!singleton.isInitialized()) singleton.init(param);}
@kd304: I was trying to spare the details, but people really hate Singleton. :-) I guess I was a bit dishonest as well since what I really needed wasn't technically a Singleton, as oxbow_lakes and many others have pointed out.
A: 

In your example you are not using a singleton. Notice that if you do the following (assuming that the Singleton.getInstance was actually static):

Singleton obj1 = Singleton.getInstance(3);
Singleton obj2 = Singleton.getInstance(4);

Then the obj2.x's values is 3, not 4. If you need to do this, make it a plain class. If the number of values is small and fixed, you can consider using an enum. If you are having problem with excessive object generation (which is usually not the case), then you can consider caching values (and check sources or get help with that, as it is obvious how to build caches without the danger of memory leaks).

You also might want to read this article as singletons can be very easily overused.

Kathy Van Stone
A: 

Singleton is, of course, an "anti-pattern" (assuming a definition of a static with variable state).

If you want a fixed set of immutable value objects, then enums are the way to go. For a large, possibly open-ended set of values, you can use a Repository of some form - usually based on a Map implementation. Of course, when you are dealing with statics be careful with threading (either synchronise sufficiently widely or use a ConcurrentMap either checking that another thread hasn't beaten you or use some form of futures).

Tom Hawtin - tackline
Only an anti-pattern if used incorrectly, though that is the definition of an anti-pattern. Just because you have seen them where they didn't belong in the past does not mean that they do not have a place.
geowa4
The correct use of a singleton is to demonstrate incompetent code.
Tom Hawtin - tackline
+1  A: 

I'll make my point very clear: a singleton with parameters is not a singleton.

A singleton, by definition, is an object you want to be instantiated no more than once. If you are trying to feed parameters to the constructor, what is the point of the singleton?

You have two options. If you want your singleton to be initialized with some data, you may load it with data after instantiation, like so:

SingletonObj singleton = SingletonObj.getInstance();
singleton.init(paramA, paramB); // init the object with data

If the operation your singleton is performing is recurring, and with different parameters every time, you might as well pass the parameters to the main method being executed:

SingletonObj singleton = SingletonObj.getInstance();
singleton.doSomething(paramA, paramB); // pass parameters on execution

In any case, instantiation will always be parameter-less. Otherwise your singleton is not a singleton.

Yuval A
+1 This is how I'd probably do it when coding. In C#, I'd just use properties though. Java, probably like this.
Zack
Please see my updated post.
A: 

There's a discussion on what's bad and good about singletons (with many good links) here - http://c2.com/cgi/wiki?SingletonsAreEvil (don't let the title deceive you - it is fairly impartial)

I'd suggest using dependency injection to control the composition of your application instead of structural patterns such as factories and singleton.

George
A: 

Modification of Singleton pattern that uses Bill Pugh's initialization on demand holder idiom. This is thread safe without the overhead of specialized language constructs (i.e. volatile or synchronized):

public final class RInterfaceHL {

    /**
     * Private constructor prevents instantiation from other classes.
     */
    private RInterfaceHL() { }

    /**
     * R REPL (read-evaluate-parse loop) handler.
     */
    private static RMainLoopCallbacks rloopHandler = null;

    /**
     * SingletonHolder is loaded, and the static initializer executed, 
     * on the first execution of Singleton.getInstance() or the first 
     * access to SingletonHolder.INSTANCE, not before.
     */
    private static final class SingletonHolder {

        /**
         * Singleton instance, with static initializer.
         */
        private static final RInterfaceHL INSTANCE = initRInterfaceHL();

        /**
         * Initialize RInterfaceHL singleton instance using rLoopHandler from
         * outer class.
         * 
         * @return RInterfaceHL instance
         */
        private static RInterfaceHL initRInterfaceHL() {
            try {
                return new RInterfaceHL(rloopHandler);
            } catch (REngineException e) {
                // a static initializer cannot throw exceptions
                // but it can throw an ExceptionInInitializerError
                throw new ExceptionInInitializerError(e);
            }
        }

        /**
         * Prevent instantiation.
         */
        private SingletonHolder() {
        }

        /**
         * Get singleton RInterfaceHL.
         * 
         * @return RInterfaceHL singleton.
         */
        public static RInterfaceHL getInstance() {
            return SingletonHolder.INSTANCE;
        }

    }

    /**
     * Return the singleton instance of RInterfaceHL. Only the first call to
     * this will establish the rloopHandler.
     * 
     * @param rloopHandler
     *            R REPL handler supplied by client.
     * @return RInterfaceHL singleton instance
     * @throws REngineException
     *             if REngine cannot be created
     */
    public static RInterfaceHL getInstance(RMainLoopCallbacks rloopHandler)
            throws REngineException {
        RInterfaceHL.rloopHandler = rloopHandler;

        RInterfaceHL instance = null;

        try {
            instance = SingletonHolder.getInstance();
        } catch (ExceptionInInitializerError e) {

            // rethrow exception that occurred in the initializer
            // so our caller can deal with it
            Throwable exceptionInInit = e.getCause();
            throw new REngineException(null, exceptionInInit.getMessage());
        }

        return instance;
    }

    /**
     * org.rosuda.REngine.REngine high level R interface.
     */
    private REngine rosudaEngine = null;

    /**
     * Construct new RInterfaceHL. Only ever gets called once by
     * {@link SingletonHolder.initRInterfaceHL}.
     * 
     * @param rloopHandler
     *            R REPL handler supplied by client.
     * @throws REngineException
     *             if R cannot be loaded.
     */
    private RInterfaceHL(RMainLoopCallbacks rloopHandler)
            throws REngineException {

        // tell Rengine code not to die if it can't
        // load the JRI native DLLs. This allows
        // us to catch the UnsatisfiedLinkError
        // ourselves
        System.setProperty("jri.ignore.ule", "yes");

        rosudaEngine = new JRIEngine(new String[] { "--no-save" }, rloopHandler);
    }
}
tukushan
A: 

Another reason Singletons are an anti-pattern is that if written according to recommendations, with private constructor, they are very hard to subclass and configure to use in certain unit tests. Would be required in maintaining legacy code, for example.

Josef B.