Is it a bad practice to create a global factory?

Question

I am studing the importance of the design pattern and I am not able to understand if could be a bad behavior to create a global static factory class (maybe using singleton?)

I put an example hoping to make it clearer.

Put that I'm creating a simple game, it could be mastermind or battleship or whatever. I could have human players and a bot players classes, then more field classes, then more pieces classes. Each of these has its own factory method.

Could be a bad practice (against SOLID) to create a static class (or singleton) that create each object? So a single class with a method that create a player, a method that create a field, a method that create a pieces and so on all in the same class?

Answer 1

I'd give two answers : one general to the title, one with your specific case :

Answer to the title

By default, I'd say yes it is a bad practice.

However it doesn't mean that there is absolutely no single reason where a multi-class (so not a fully global) factory could be that bad.

Let's say that your factory isn't global but limited to everything that can move on the map. This factory is multi class but yet not exactly global.

You could for instance need to initialize all those classes with some common parameters and instead of bringing along all those parameters across all functions, having them all in your factory and pass them down to the instance of classes. It is basically what a DI container is doing.

The trap here is that there is already stuff doing this : DI container so why repeat yourself ? The basic usage of DI container is often to create "Singleton" Service class (Singlton between quote because you will instantiate only one thanks to the DI container but you're not using the Singleton pattern, making testing easier).

But DI container can also handle the creation of objects on demand, this might have a cost on performance if you need really a lof of instances. And this is where your factory might be usefull : create your factory from the DI container in order to have everything you need. Use your factory to create everything you need.

Answer to the content

I'am not sure that what you are asking is only a simple multi-class factory (I think your question is what we call an XY problem here). To me it seems like the goals of this class is to be used to initialize everything in your game. For instance let's suppose we're going for chess, you will need to initialise 8 pawns, 1 queen, 1 king, .... You could have your global factory and one other class performing the initialization through calling your factory.

But I do not think this would be the proper distribution of responsabilities. I think that one class in charge of fully initialize the game by instantiating everything needed would be more appropriate for chess.

Answer 2

Good is a value judgement... so what are your values?

Singletons

Singletons normally get a bad reputation because:

• they are generally accessible across a code base. This essentially adds them as a dependency of all of your code.
• they are generally difficult to mock/stub. This makes unit testing much harder to perform well.
• may not have initialised yet. This requires validation logic to be distributed throughout the code base, and perhaps even recovery logic (such as actually initialising it).
• a singleton constraint is contextual. Outside of that context there may be several variants.
• there is usually a superior means to achieve the same objective, such as Dependency Injection.

Generally speaking these problems lead to poorer overall code.

If you do choose to implement a singleton:

• reduce the reach of the singleton to just those units of code that must access it.
• provide a mechanism or swapping the singleton with a test mock
• ensure that the singleton is initialised before any unit of code that depends on it runs, to reduce validation and initialisation logic spread.
• ensure you keep an eye on it.

Multi-Factories

Depends on how you implement these as to whether or not they are SOLID compliant. Just pointing it out - SOLID is a set of principles - not the end goal of software.

The question you really need to ask is why the code cannot simply allocate the type directly?

A few circumstances stand out:

• the code is a general algorithm. In which case use a specialised factory passed into the algorithm.
• the type is complicated to construct. In which case create and call the factory directly.
• the type is complicated and the factory is not much less difficult to construct. In which case prefer to have this factory passed in.

The only factory which is generally sensible to make global, is an Inversion of Control Container (an IoC). This is a complicated to create factory (usually configuration driven), where the overhead of passing it in to every function call, or creating it on demand, might be more than the overhead of having a static global singleton. This is a trade-off, but one which sometimes makes sense.

Answer 3

Yes, "create all the things" is a violation of the single responsibility principle. Making it global is going to couple all of your code to that one god object. And making a singleton will harm your testing, probably make singleplayer vs multiplayer harder, and generally cause you all sorts of problems with no benefit.

This certainly seems like a case of pattern obsession from the little you've told us. If you want to make an object, just make an object. If you want to break a dependency, then take the object as input (via an interface if appropriate) rather than making it directly.

Answer 4

It is bad practice to create such an object or static class, for the similar reasons why the "Service Locator" pattern is a bad idea. You can find more information about that here: What's the difference between using dependency injection with a container and using a service locator?

If you need a Thing object, why not just do new Thing()? If you decide to use factories to create such objects, I would advise you to create a specialized factory for Things and other factories for other types of objects.

Answer 5

In short: Nope.

Singleton factory is essentially equivalent to having a constructor or a static create method.

The benefit comes when the factory is configurable (as it should be) and each singleton factory represent one of the most commonly used configurations.

For instance for your Battleship game

class ShipFactory {
    public static readonly ShipFactory Boat = new ShipFactory{Class="Boat", Shape= new (int, int)[] { (0,0), (1,0) } };
    public static readonly ShipFactory Carrier = new ShipFactory{Class="Carrier", Shape= new (int, int)[] { (0,0), (1,0), (2,0), (2,1), (3,1), (4,1) } };

    public string Class;
    public (int, int)[] Shape;

    public Ship CreateShip(int x, int y, int orientation)
       => null;
}

And with this approach you can make a "ship editor" to your game.