Dealing with a Factory that uses both a Strategy and Repository

Question

I've got a question around the best way to refactor a Factory like this:

I am working on a side-project and have ended up in a position whereby I have an OrderProductFactory that requires a strategy pattern for payment but then also a repository which dictates where my OrderProducts are stored so for example:

final readonly class OrderProductFactory
{
    public static function create(string $orderProductPaymentStrategyType, string $repositoryType, array $repositoryParams): OrderProduct
    {
        return OrderProduct::for(
            self::createOrderProductPaymentStrategy($orderProductPaymentStrategyType),
            self::createOrderRepository($repositoryType, $repositoryParams)
        );
    }

    private static function createOrderProductPaymentStrategy(string $type): OrderProductPaymentStrategy
    {
        return match ($type) {
            CardPayment::name() => new OrderCardPayment(),
            default => throw new InvalidArgumentException('Unsupported OrderProductPaymentStrategy type: ' . $type),
        };
    }

    private static function createOrderRepository(string $type, array $params): OrderRepository
    {
        return match ($type) {
            SqliteOrderRepository::name() => new SqliteOrderRepository($params['database_path'] ?? throw new InvalidArgumentException('Invalid File Path for SqliteOrderRepository')),
            default => throw new InvalidArgumentException('Unsupported OrderRepository type: ' . $type),
        };
    }
}

This works in a way as I can easily swap out what creates my order product so I can call it with usage such as:

$orderProduct = OrderProductFactory::create('card-payment', 'sqlite'); 

However this feels incorrect to me as it is violating SRP as while my OrderProductFactory cares about what it's payment strategy is and also how it's persisted.

It feels incorrect that it is responsible for:

• Creating the OrderProduct
• Creating the correct Payment strategy
• Handling which repository to load
• I also dislike that a magic array $params is used

Is there a better way I can approach this perhaps with a Builder or a Factory of Factories pattern? Open to improvements. Thanks

Answer 1

If you want do that in similar way to your code just use Enums (that simplifies argument validation and takes implementation listing outside Factory class).

final readonly class OrderProductFactory
{
    public static function create(Payment $payment, Repository $repository): OrderProduct
    {
        return OrderProduct::for(
            $payment->getPaymentStrategy(),
            $repository->getRepository(),
        );
    }
}

enum Payment
{
    case CardPayment;

    public function getPaymentStrategy(): OrderProductPaymentStrategy
    {
        return match ($this) {
            self::CardPayment => new OrderCardPayment(),
        };
    }
}

enum Repository
{
    case SqliteOrderRepository;

    public function getRepository(array $params): OrderRepository
    {
        return match ($this) {
            self::SqliteOrderRepository => new SqliteOrderRepository(),
        };
    }
}

And the usage:

$orderProduct = OrderProductFactory::create(Payment::CardPayment, Repository::Sqlite);

But in my opinion whole Factory class is redundant, since OrderProduct::for is in fact a Factory itself. Just define Payment and Repository as interfaces (what is in fact an essence of Strategy Pattern) and feed it with context-appropriate implementations.

class OrderProduct
{
    public static function for(Payment $payment, Repository $repository): self
    {
        return new OrderProduct($payment, $repository);
    }
}

interface Payment
{
    // some methods
}

class OrderCardPayment implements Payment
{
    // some implementation
}

interface Repository
{
    // some methods
}

class Sqlite implements Repository
{
    // some implementation
}

And usage:

$orderProduct = OrderProductFactory::for(
    payment: new OrderCardPayment(), 
    repository: new Sqlite()
);

In terms of "magic $params", I don't see why the database path should be passed as an argument. Either it should be set in app configuration or it should be used as an argument of different Repository interface implementations.