OOP Best practices: Is there any reason to separate out Factory functionality from an abstract base class?

Question

Consider the following python3 code:

from abc import ABC, abstractmethod

class Food(ABC):
    _food_factory_map = {}
    _recipes = {}

    @classmethod
    def getFood(cls, foodName):
        return cls._food_factory_map[foodName]()

    @classmethod
    def registerFood(cls, foodName, recipe):
        def callable(foodClass):
            cls._food_factory_map[foodName] = foodClass
            cls._recipes[foodName] = recipe
            return foodClass
        return callable

    def __init__(self):
        print("Starting Food prep")

    @abstractmethod
    def prepareFood(self):
        ...

    def serveFood(self):
        food = self.prepareFood()
        print(food)

@Food.registerFood('Burger', 'Burger Recipe')
class Burger(Food):
    def prepareFood(self):
        return 'Burger is ready!'

@Food.registerFood('Pizza', 'Pizza Recipe')
class Pizza(Food):
    def prepareFood(self):
        return 'Pizza is ready!'

if __name__ == '__main__':
    fs = ('Burger', 'Pizza')
    for f in fs:
        print(f"Food Recipe: {Food._recipes[f]}")
        food = Food.getFood(f)
        food.serveFood()

Here the abstract base class Food has class methods that implement the Factory design pattern. A client can register their own concrete classes with registerFood and retrieve them with getFood. The abstract base class also implements other abstract and concrete methods to define the interface.

One can also separate out the two class methods (getFood and registerFood) and the two class members (_food_factory_map and _recipes) into a class of it's own (FoodFactory)

class FoodFactory(ABC):
    _food_factory_map = {}
    _recipes = {}

    @classmethod
    def getFood(cls, foodName):
        return cls._food_factory_map[foodName]()

    @classmethod
    def registerFood(cls, foodName, recipe):
        def callable(foodClass):
            cls._food_factory_map[foodName] = foodClass
            cls._recipes[foodName] = recipe
            return foodClass
        return callable

All the client code will then change to using FoodFactory instead of Food.

@FoodFactory.registerFood('Pizza', 'Pizza Recipe')
class Pizza(Food):
    def prepareFood(self):
        return 'Pizza is ready!'

if __name__ == '__main__':
    fs = ('Burger', 'Pizza')
    for f in fs:
        print(f"Food Recipe: {FoodFactory._recipes[f]}")
        food = FoodFactory.getFood(f)
        food.serveFood()

Which of these implementations is better and why? Or is it perfectly reasonable to use either? Is it language dependent?

One advantage I can think off is that with the second method, one can override the __new__ of FoodFactory to directly return an instance of the appropriate food rather than calling a separate getFood method. But that difference seems largely cosmetic.

Answer 1

Your blanket question

OOP Best practices: Is there any reason to separate out Factory functionality from an abstract base class?

A factory is warranted when a constructor is no longer the appropriate solution to the problem at hand. So you could argue that a constructor is a really simple factory, and that you only need to consider a factory (i.e. the separate class) when the construction logic gets complicated enough to warrant separation from the factory's product (i.e. the original class).

So, to answer your blanket question if there is any reason to every separate it out, the answer is yes. Specifically, in cases where the construction logic is sufficiently complex to warrant it, or e.g. when you need an injectable dependency which is able to generate products on the fly.

---

Your concrete example

It doesn't make semantical sense for Food to know a collection of recipes. That's meta-knowledge. Analogously, your car does not contain its own construction blueprint, let alone that of many other cars was well.

My intuition is telling me that this is a composition-over-inheritance violation. It makes more sense for the collection of recipes to live in a dedicated class (let's call it RecipeBook).

Inheritance is the wrong construct to use here. You're not making any use of the polymorphism that inheritance provides. A Burger is not a collection of recipes, and that's what your inheritance is currently claiming.

I'm no Python dev, so I don't feel comfortable writing an improved version of your snippet.

Answer 2

I'd say the main reason here is that you have applied a singleton pattern to your factory.

You can never have a different factory.

You can never mock the factory.