CODEWITHSUNDEEP
javamultithreading
Subhabrata Mondal
Subhabrata Mondal

Reputation: 536

Thread Safety of Factory Class

I have a factory class let say ApppleFactory that creates a new instance of Apple by invoking getApple() method. There is only one shared instance of ApppleFactory.

class ApppleFactory {
    private static Apple apple = null;

    private ApppleFactory() {
    }

    public static Apple getApple() {
        apple = new Apple();
        //do some other work on this apple instance
        return apple; //finally return it
    }
}

Now if multiple threads access this getApple() and work with this mutable data apple then there could be a chance of unsafety. How to overcome this problem?

Upvotes: 0

Views: 288

Answers (2)

davidxxx
davidxxx

Reputation: 131456

The apple field is not required as you create at each time a new instance in getApple() . So you can remove it :

class ApppleFactory {

    private ApppleFactory() {
    }


    public static Apple getApple() {
        Apple  apple = new Apple();
        //do some other work on this apple instance
        return apple; //finally return it
    }
}

Now if your need changes and you want to reuse this field somewhere else in your factory class, the class would be so mutable and you should so use explicit synchronization to create and retrieve Apple instances.
For example with your actual implementation relying on static modifiers:

class ApppleFactory {

    private static Apple apple;

    private ApppleFactory() {
    }

    public static synchronized Apple getApple() {
        apple = new Apple();
        //do some other work on this apple instance
        return apple; //finally return it
    }

    public static synchronized void consumeApple() {
        apple.eatByMe();
    }

}

Note that providing only static methods in a factory couples strongly the clients of the class to the class. As alternative you could change clients methods into instance methods by using the classical singleton, the enum singleton pattern or a dependency injection container.

Upvotes: 2

OldCurmudgeon
OldCurmudgeon

Reputation: 65851

Clearly the static field is the problem. Just get rid of it.

class ApppleFactory {
    public Apple getApple() {
        Apple apple = new Apple();
        //do some other work on this apple instance
        return apple; //finally return it
    }
}

Upvotes: 6

Related Questions