Should a standard factory always generate a new instance?

Question

According to my understanding on factory-method, factory should always return a new instance, meaning no cache, So in essence, every time when the factory method is called, there should always be a new instance returned, otherwise, it would not be a factory method pattern. For instance, the following code is not a factory:

public class UserManagementActionListenerFactory extends AbstractListenerFactory {
    private static final String ENABLE_USER = "enableUser";
    private static final String DISABLE_USER = "disableUser";
    private static final String DELETE_USER = "deleteUser";
    private static final String CONVERT_USER = "convertUser";
    private Map<String,ActionListener> mapper;
    public ActionListenerMapping() {
        mapper = registerListeners();
    }
    private Map<String, ActionListener> registerListeners() {
        Map<String,ActionListener> actionListenerMap = new HashMap<>();
        actionListenerMap.put(ENABLE_USER, new EnableActionListener());
        actionListenerMap.put(DISABLE_USER, new DisableActionListener());
        actionListenerMap.put(DELETE_USER, new DeleteActionListener());
        actionListenerMap.put(CONVERT_USER, new ConvertUserActionListener());
        return actionListenerMap;
    }
    @Override
    ActionListener getActionListener(String s) {
        return mapper.get(s);
    }
}

The UserManagementActionListenerFactory is used in a bean called UserManagementBean:

public class UserManagementBean {
    AbstractActionListenerFactory factory;
    @PostConstruct
    public init() {
        factory = new UserManagementActionListenerFactory();
    }
    //... irelevant code
    public void handleActionEvent(ActionEvent e) {
        String componentId = e.getComponent().getId();
        factory.getActionListener(componentId).processAction(e);
    }
}

As this ActionListenerMapper initialized all the concrete subclasses of ActionListener when being constructed, then every time the getActionListener(s) is called, it always returns a cached ActionListener instance. Please tell here if my understanding is wrong

However, in first item of the book Effective Java:

A second advantage of static factory methods is that, unlike constructors, they are not required to create a new new object each time they're invoked.

Moreover, Effective Java also told:

Note that a static factory method is not the same as the Factory Method pattern from Design Patterns [Gamma95]

Question: Usually should a factory method always return a new instance instance?

Answer 1

While the other answers properly address most of the concerns regarding the usage of a factory and the factory pattern in general, I would only like to add that your class is not necessarily a factory. That is, not until you can reason properly about why it is a factory.

A factory is not a factory because the class name ends in -Factory, obviously. By all means, your question and the answers already cover issues regarding the expected behavior of a factory, but just making a class that provides access to something (cached or not cached) does not make it a factory.

Based on your code (without any more context), I cannot see why a class/interface method, which receives a string and returns action listeners, would be considered a factory. I don't see any difference between your class and an identical class named ActionListenerRepository with a getActionListenerByID(string) instead of getActionListener(string) (or even the same). Besides, callers of the method (apparently, from your code) don't really know how, when or why the returned instances were created/added.

Patterns are good, knowing how to utilize them properly even more so, but you must not skip semantics.

So, what you should clarify is: Why is this a factory to you and your object model?

Edit

After the question has been edited to include a specific use-case, and after the exchange in the comments, an update is made to the answer.

Let's try to look at it from the code reviewer's perspective. Coding is all about meaning and semantics. When we write high-level code, think of it as giving instructions to someone (i.e. not to the computer). When we refer to a factory, a factory comes to mind. A factory produces things, it does not, usually, store things. It produces them. So when a reviewer sees a factory that is, in fact, a storage place, instead, the boundary is crossed between intended and expected meaning. Mixing up the two violates the principle of least astonishment, which is "jargon" for not getting what you expected.

What the "client" expects is not all that important, because it is just as easy for the client to get something from the factory and cache it themselves. But think about it in the intuitive sense. Would you request something from a factory each time you need to use it?

In short, the primary reason your factory is not a factory is that you are neither building it as such, nor using it as such. The expected usage of a factory is to:

• Request a thing every now and then (as in "not every time").

• Expect something new most of the time.

• Keep the returned object for further use.

Your class is used in a totally opposite way. It requests the factory's result at every use, expects the same object anyway, because who would like to get different action listeners upon each request, and does not keep them stored inside the client class.

Last, but not least

Because you also mentioned caching, factories are not paired with caching in the way you seem to think they are. Factories don't typically cache objects for multiple returns. Factories cache new instances for immediate returns. This means that, while the user of an application is doing other things, a factory can spawn a couple of tasks/threads that construct new instances, so that the next time a request is made, the new instance is at the ready, hence immediately returned, and, of course, (potentially forever) forgotten by the factory (unless you are doing instance counting, etc. for whatever reason).

So, caching, in the context of a factory, has a more useful meaning as having pre-constructed objects to hand out immediately, not as handing out the same objects each time. When you pass an instance reference to someone, you immediately forfeit "total" ownership. Now someone else can play with your object, so, I am asking you, as a factory: "Are these objects yours after you give them away?"

Answer 2

No, Joshua Bloch is right (as he usually is). A factory or factory method need not create a new instance for every call. That is, in fact, one of the advantages of factories.

Look at it this way: the point of a factory is to relieve a client programmer from having to understand the exact type hierarchy of a resource. This means that you can replace your sturdy StandardStructure with a snazzy ImprovedStructure and roll it out without having to change any client code. This is a good thing.

Likewise, the customer of a factory doesn't have to understand the details of the lifecycle of your objects. They may be good for one client only, or they may be smart enough to serve many masters. They may be able to deal with any number of calls, or have a resource (maybe bits of entropy) that decays with use. No matter - the factory knows how to manage object lifetimes correctly, meaning that the client programmer doesn't have to understand it. The result is the same win in maintainability as for the details of your type hierarchy.

Answer 3

I see two different possible interpretations of this question:

1. should a factory method always return a new instance instance - because the term "factory method" is broadly accepted in software development to describe functions which behave like this?

So the solution would be here to give the method a different name other than one containing the term "factory"?

Or

2. should a factory method always return a new instance instance - because if it does not, will this become prone to errors?

So the solution is not to write such methods?

The answer to the first question is definitely "no". As with many terms in software engineering, the name "factory method" has more than one valid meaning, there is no "mathematically strict definition" for it. Factories are expected to choose between different subtypes and deliver objects one otherwise had to create by new, but if they call new by themselves, or are implemented differently should not matter. You can define whatever convention you like for your factories, and if you see a valid reason why you think it is necessary in your system to make certain factories to always create new instances, then go ahead and establish this convention in your team. Or you can allow factory method to behave differently, there is no "law" which forbids this.

The answer to the second question is: it is usually safe for a factory to deliver the same instance of an immutable object more than once if the input values of the factory method are the same. I guess in your example above, every ActionListener is either immutable or treated in an immutable fashion. If the returned objects are not immutable, returning the same instance twice could lead to all kind of unwanted side effects, so it would be less error prone to deliver always a newly created instance.

There is another design where object reuse may be ok even for mutable objects: that's when your factory manages some resource pool (for example, a thread pool), and users have to allocate "resource objects" from there and return the used objects after usage explicitly. Of course, it is debatable if for such cases the name "factory" would still be a good fit.

Let me add a note about your example code: from the viewpoint of a caller of that function getActionListener, it does most probably not matter if a call to this function always produces a new object, or if the same (immutable) object is returned on two calls with the same input parameter. Hence if that code snippet is better called a "factory" or a "mapper" is mostly a matter of taste - the name should not depend on how the function is implemented internally.