Is this a valid usage of the Factory pattern? multiple dependencies

Question

I want to create a set of dependencies instead of injecting them everywhere. Would the factory pattern support this idea? Or is there another pattern for dealing with it?

For example:

class PassportCheckFactory {
    protected $clientInstance;
    protected $responseInstance;

    public function buildDependancies() : bool
    {
        $this->clientInstance       = new PassportCheckSoapClient;
        $this->responseInstance    = new PassportCheckResponse;

        return true;
    }

    public function getResponseInstance()
    {
        return $this->responseInstance;
    }

    public function getClientInstance()
    {
        return $this->clientInstance;
    }
}

This creates and holds the classes we would use, so we wouldn't need to inject them.

For example, we can do this

$request = new WhateverRequestClass;
$factory = (new PassportCheckFactory)->buildDependancies();
$service = new PassportCheckService($request, $factory);
$response = $service->execute();

instead of:

$request = new WhateverRequestClass;
$service = new PassportCheckService($request, new PassportCheckSoapClient, new PassportCheckResponse);
$response = $service->execute();

I would recommend going with the last example (the _"instead of"_) since that's proper dependency injection. It will also be much easier to test (passing mock objects etc.). In your other examples, you've basically turned your factory class into a service locator. Your factory will also instantiate all classes regardless if you need them for that request or not. — M. Eriksson, Feb 21 '18 at 23:02
I would also recommend that you look into some IoC container that can make your code cleaner and easier to manage, like Pimple, Illuminate\Container or similar. Here's a stackoverflow post talking about dependency injection and containers: https://stackoverflow.com/questions/18562752/understanding-ioc-containers-and-dependency-injection — M. Eriksson, Feb 21 '18 at 23:08
thanks @MagnusEriksson If I use an IoC container, would that tightly couple my code to that container code. Lets say I needed to use the feature inside a legacy codebase, where the requirements don't support the container, I'd be stuck? — Dave Jones, Feb 22 '18 at 16:24

score 0 · Accepted Answer · answered Feb 22 '18 at 15:04

Your approach makes sense, if you want to support multiple CheckServices. If PassportCheckService is the only one, the factory / service locator / specialised container from your example is just adding overhead.

$request = new WhateverRequestClass;
$service = new PassportCheckService($request, new PassportCheckSoapClient, new PassportCheckResponse);
$response = $service->execute();

is actually the best solution for a stand-alone service in terms of readability, maintainability and testabilty.

Multiple CheckServices

However, if you want to support multiple services, extracting the composition of the service into its own class brings benefits.

class CheckServiceFactory
{
    public static function getService(Request $request, string $serviceType): CheckService
    {
        $type = ucfirst(strtolower($serviceType));

        $serviceClass  = $type . "CheckService";
        $clientClass   = $type . "CheckSoapClient";
        $responseClass = $type . "CheckResponse";

        return new $serviceClass($request, new $clientClass, new $responseClass);
    }
}

Of course, the generation of the classnames depends on your naming scheme.

Calling a specific service would look like this:

$request = new WhateverRequestClass;
$service = CheckServiceFactory::getService($request, 'Passport');
$response = $service->execute();

Proposed Refactoring

Beside of the things above, I'd recommend to refactor the service class itself, so the request gets out of the constructor. That changes the usage to:

$request = new WhateverRequestClass;
$service = CheckServiceFactory::getService('Passport');
$response = $service->handle($request);

or in case of a single service:

$request = new WhateverRequestClass;
$service = new PassportCheckService(new PassportCheckSoapClient, new PassportCheckResponse);
$response = $service->handle($request);

which actually looks much more straight forward.

superb stuff, I owe you a beer! Do you think for testing the service class, I could simply bypass the factory (since its static) and create the service with a mocked client injected? — Dave Jones, Feb 22 '18 at 16:12
I made the factory static for shortness sake. It is better to keep it nonstatic. But you could create a 'PassportTest' service, which is built by the same factory. — nibra, Feb 23 '18 at 02:13

Is this a valid usage of the Factory pattern? multiple dependencies

1 Answers1

Multiple CheckServices

Proposed Refactoring