Java Abstract Factories and Singletons

Question

I would like to implement an example of abstract factory, however concrete factories must be served as singletons.

Giving the example of Mr. Banas here : http://www.newthinktank.com/2012/09/abstract-factory-design-pattern/ I should modify UFOEnemyShipFactory and UFOBossEnemyShipFactory right ?

I tried something for UFOEnemyShipFactory but I'm not sure to be right :

public class UFOEnemyShipFactory implements EnemyShipFactory{

    private UFOEnemyShipFactory(){};

    private static UFOEnemyShipFactory firstInstance = null;

    public static UFOEnemyShipFactory getInstance(){
    
        if(firstInstance == null){
        
            synchronized(UFOEnemyShipFactory.class){
            
                if(firstInstance == null){
                
                    firstInstance = new UFOEnemyShipFactory();
                
                }
            
            }
        
        }
    
        return firstInstance;
    
    }

    public ESWeapon addESGun(){
        return new ESUFOGun();
    }

    public ESEngine addESEingine() {
        return new ESUFOEngine();
    }

It seems a bit weird, I think that I'm not applying the needed modification in the correct class. If I'm totally wrong could you please give me a brief explanation (why am I wrong and which class(es) I have to modify and why ?)

Thanks In advance.

Answer 1

I'm afraid I do not have complete info on your AbstractFactory question, but I can help you on your singleton issues...

Usually when you attempt the singleton design pattern you specify a private constructor so others cannot use the default constructor to create their own instances.

As an additional observation, you have dropped right into the classic "double-check" anti-pattern (something to not do). I wrote more on that here: What's wrong with the following getInstance() method Basically, don't check outside of a sync block, synchronize, then check again. It looks like it is safe, but it is not. For this:

public static UFOEnemyShipFactory getInstance(){
    if(firstInstance == null){
        synchronized(UFOEnemyShipFactory.class){
            if(firstInstance == null){
                firstInstance = new UFOEnemyShipFactory();
            }
        }
    }
    return firstInstance;
}

write either this:

public static synchronized UFOEnemyShipFactory getInstance(){
    if(firstInstance == null){
                firstInstance = new UFOEnemyShipFactory();
    }
    return firstInstance;
}

or many would argue better:

private static final UFOEnemyShipFactory firstInstance = new UFOEnemyShipFactory();

public static UFOEnemyShipFactory getInstance(){
    return firstInstance;
}

Also a style point. While in theory you can name things like you wish, "firstInstance" begs questions. Is there a "secondInstance"? But, "instance" (at least in the java sense) normally means "the one and only". Better just "instance". If your idea is to have 2 singletons hanging around somewhere, but they are distinct, you might name them, ufoBossInstance and ufoGruntInstance, for example following the naming of the references page.