Creating many objects using composition when the containing class is a singleton pattern

Question

java version 1.7.0_65

I have a singleton design pattern class. That will always return the same instance that is initially created.

However, the problem I have is this class needs to create many other objects from another class. I have used composition for this (instance of POI class in ArlabFacade). From this singleton instance the client should be able to create many POI objects. And I don't want to expose the internal workings of the POI class, everything has to go through the singleton instance.

private static ArlabFacade mArlabFacade = null;
private POI mPoi; /* Should be able to create many object of this class */

private ArlabFacade(Context context) {     
        /* initialize properties */
        mContext = context;

        mPoi = null;
    }

public static ArlabFacade getInstance(Context context) {

        /* Create a synchronised singleton instance */
        ReentrantLock lock = new ReentrantLock();
        lock.lock();
        if(mArlabFacade == null) {
            mArlabFacade = new ArlabFacade(context);
        }
        lock.unlock();

        return mArlabFacade;
    }

I have tried doing something like this, but it has 2 problems.

1) I don't want to return the class instance of POI
2) because I only have a single instance the mPoi will be overwritten by the next client call to this function. 

This function will just overwrite:

 public POI createNewPOI() {
        return mPoi = new POI();
    }

Is there any design patterns that address this problem?

Answer 1

I'm a bit confused what exactly you are trying to achieve. It looks like you want a Factory, i.e. a class that hides how to create objects of a certain class. There is no need for a Singleton in that case, unless you have other reasons.

As to your problems:

1. You are nor returning a class instance, but an object of that class. I thought that was the whole point: creating that POI object ant returning it. I guess there is some confusion about the nomenclature, so please explain what you mean by class instance, and why you don't want to return it.

2. In your factory method createNewPOI() you just overwrite your reference to the last created POI object, not the object itself. Unless your factory class (resp. your Singleton) is doing something with the POI object itself, there is no need to keep a reference. You return the object to the caller of the method. After that you can just forget it:

.

public POI createNewPOI() {
    return new POI();
}

There is one more problem in your code: Your locking in the getInstance() method won't work. For a ReentrantLock to do its job it has to be shared between multiple threads. In your case each thread creates its own copy of the lock, without knowing of the other theads.

The easiest way is to just make the method synchronized:

public static synchronized ArlabFacade getInstance(Context context) {

    if(mArlabFacade == null) {
        mArlabFacade = new ArlabFacade(context);
    }

    return mArlabFacade;
}

Answer 2

Take a look at: What is so bad about singletons?

You should only use code patterns if you have a reason. Ex: opular patterns and reasons to use them are:

Creational Patterns

• Abstract Factory Creates an instance of several families of classes
• Builder Separates object construction from its representation
• Factory Method Creates an instance of several derived classes
• Prototype A fully initialized instance to be copied or cloned
• Singleton A class of which only a single instance can exist

Structural Patterns

• Adapter Match interfaces of different classes
• Bridge Separates an object’s interface from its implementation
• Composite A tree structure of simple and composite objects
• Decorator Add responsibilities to objects dynamically
• Facade A single class that represents an entire subsystem
• Flyweight A fine-grained instance used for efficient sharing
• Proxy An object representing another object

Behavioral Patterns

• Chain of Resp. A way of passing a request between a chain of objects
• Command Encapsulate a command request as an object
• Interpreter A way to include language elements in a program
• Iterator Sequentially access the elements of a collection
• Mediator Defines simplified communication between classes
• Memento Capture and restore an object's internal state
• Observer A way of notifying change to a number of classes
• State Alter an object's behavior when its state changes
• Strategy Encapsulates an algorithm inside a class
• Template Method Defer the exact steps of an algorithm to a subclass
• Visitor Defines a new operation to a class without change

source : dofactory

Answer 3

Quite simple, but first of all, please note, that lock must be created within the static context, so that every thread will use the same lock instance (there will be no synchronization at all if using the different instance for each thread)

Now, here is the code

public class ArlabFacade {

    private static ArlabFacade mArlabFacade = null;

    /* Create a synchronised singleton instance */
    private static final ReentrantLock lock = new ReentrantLock();

    private ArlabFacade(Context context) {     
        /* initialize properties */
        mContext = context;
    }

    public static ArlabFacade getInstance(Context context) {
        lock.lock();
        if(mArlabFacade == null) {
            mArlabFacade = new ArlabFacade(context);
        }
        lock.unlock();
        return mArlabFacade;
    }

    public NotAPOI createNewPOI() {
        return new NotAPOIImpl(new POI());
    }

    public void doSomething(NotAPOI val) {
        if(!(val instanceof NotAPOIImpl)) {
            throw new IllegalArgumentException("Illegal implementation of NotAPOI");
        }
        NotAPOIImpl impl = (NotAPOIImpl) val;
        POI poi = val.poi;
        // do something with poi here
    }

    private static class NotAPOIImpl implements NotAPOI {
        private POI poi;
        private NotAPOIImpl(POI poi) {
            this.poi = poi;
        }
    }
}

// As you don't like to expose the POI just hide it behind the interface
public interface NotAPOI {

}

Another possible solution is to allow to work with POI by means of one more level of abstraction, which is more elegant

public class ArlabFacade {

    private static ArlabFacade mArlabFacade = null;

    /* Create a synchronised singleton instance */
    private static final ReentrantLock lock = new ReentrantLock();

    private ArlabFacade(Context context) {     
        /* initialize properties */
        mContext = context;
    }

    public static ArlabFacade getInstance(Context context) {
        lock.lock();
        if(mArlabFacade == null) {
            mArlabFacade = new ArlabFacade(context);
        }
        lock.unlock();
        return mArlabFacade;
    }

    public NotAPOI createNewPOI() {
        return new NotAPOIImpl(new POI());
    }

    private static class NotAPOIImpl implements NotAPOI {
        private POI poi;
        private NotAPOIImpl(POI poi) {
            this.poi = poi;
        }
        public void doSomething() {
            poi.doSomething();
        }
    }
}

// As you don't like to expose the POI just hide it behind the interface
public interface NotAPOI {
    void doSomething();
}

Answer 4

If I understand you correctly, you want all callers to get the same singleton class, but each caller to operate on his own POI Object. But this POI Object should be hidden in the Singleton-Class.

You can do it like this:

Each Callsite/Client will first have to call ArlabFacade.getInstance().generateToken() so he gets a unique Token, so he gets one POI reserved for use. And when he is done he should call releaseToken()

private static ArlabFacade mArlabFacade = null;

private HashMap<String, POI> poiMap = new ConcurrentHashMap<>();

private ArlabFacade(Context context) {     
        /* initialize properties */
        mContext = context;
    }

public static synchronized ArlabFacade getInstance(Context context) {
    /* Create a synchronised singleton instance */
    if(mArlabFacade == null) {
        mArlabFacade = new ArlabFacade(context);
    }

    return mArlabFacade;
}

private AtomicInteger uniqueStringCounter = new AtomicInteger(0);

public String createUniqueString() {
    return "TOKEN"+uniqueStringCounter.getAndIncrement();
}

public String generateToken() {
    String token = createUniqueString();
    poiMap.add(token, new POI());
}

public void releaseToken(String token) {
    poiMap.remove(token);
}

public void doStuffOnPOI(String token, int someParameter) {
    POI mPoi = poiMap.get(token);

    mPoi.doStuff(someParam);
}

Answer 5

When writing lazy-loaded (on-demand) singleton in java, you must be aware of some problems:

Double-checked locking pattern is considered unsafe in multithreaded environment.

/*
 * unsafe and broken Double-Checked Locking pattern
 */  
public class ArlabFacade {
    private ArlabFacade mArlabFacade;

    public ArlabFacade getInstance() {
        if (mArlabFacade == null) {
            synchronized(this) {
                if (mArlabFacade == null) {
                    mArlabFacade = new ArlabFacade(...);
                }
            }
        }
        return mArlabFacade;
    } 
}

Double-checked locking pattern is bad due to Java Memory Model described in language specification. See below links:

http://en.wikipedia.org/wiki/Double-checked_locking

http://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html

From this point of view safe and valid pattern is called Initialization on Demand Holder and is shown below:

Fragment from wiki: "[...] In all versions of Java, the idiom enables a safe, highly concurrent lazy initialization with good performance [...]"

See:

http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom

/*
 * fully safe and performant holder pattern 
 */ 
public class ArlabFacade {

    private static class Holder {

        private ArlabFacade instance;
        private List<POI> mPOIs;

        static {
            instance = new ArlabFacade();

            mPOIs = new ArrayList();
            mPOIs.add( new POI(...) );            
            mPOIs.add( new POI(...) );
            ....
        }
    }


    public static ArlabFacade getInstance() {
        return Holder.instance;
    }

}

Above holder pattern guarantees to be safe and performant becase static class Holder is loaded only once (JVM spec) and lazily - only when getInstance() is called for the first time.