hungl
hungl

Reputation: 101

Interface Segregation Principle: How to split big interface with a lot of optional methods

Suppose I have an interface:

interface WorkerInterface
{
    public function doCommonAction1(CommonAction1Params $params): CommonAction1Result;

    public function doCommonAction2(CommonAction2Params $params): CommonAction2Result;

    /**
     * @return void
     *
     * @throws UnsupportedMethodException
     */
    public function doSpecificAction1(SpecificAction1Params $params): SpecificAction1Result;

    /**
     * @return void
     *
     * @throws UnsupportedMethodException
     */
    public function doSpecificAction2(SpecificAction2Params $params): SpecificAction2Result;
}

The problem is that methods doSpecificAction1 and doSpecificAction2 are optional and supported not by all workers. Worker can support doCommonAction1 and doCommonAction2 only, as well as doCommonAction1, doCommonAction2 and doSpecificAction1, or doCommonAction1, doCommonAction2, doSpecificAction2, or all methods together.

Also I have a WorkerFactory:

class WorkerFactory
{
    public function createWorker(string $workerId): WorkerInterface
    {
        // worker is created here
    }
}

Then I have a controller:

class ActionController {
    public function commonAction1(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        $worker->doCommonAction1(new CommonAction1Params());
    }

    public function commonAction2(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        $worker->doCommonAction2(new CommonAction2Params());
    }

    public function specificAction1(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        try {
            $worker->doSpecificAction1(new SpecificAction1Params());
        } catch (UnsupportedMethodException $e) {
            // do something
        }
    }

    public function specificAction2(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        try {
            $worker->doSpecificAction2(new SpecificAction2Params());
        } catch (UnsupportedMethodException $e) {
            // do something
        }
    }
}

It's obvious that now my code violates Interface Segregation Principle. I would like to refactor it. OK, I try doing something like this:

interface WorkerInterface
{
    public function doCommonAction1(CommonAction1Params $params): CommonAction1Result;

    public function doCommonAction2(CommonAction2Params $params): CommonAction2Result;
}

interface SpecificAction1AwareInterface
{
    public function doSpecificAction1(SpecificAction1Params $params): SpecificAction1Result;
}

interface SpecificAction2AwareInterface
{
    public function doSpecificAction2(SpecificAction2Params $params): SpecificAction2Result;
}

So now my workers will look like this:

class Worker1 implements WorkerInterface {}

class Worker2 implements WorkerInterface, SpecificAction1AwareInterface {}

class Worker3 implements WorkerInterface, SpecificAction1AwareInterface, SpecificAction2AwareInterface {}

And now my controller changes to this:

class ActionController {
    public function commonAction1(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        $worker->doCommonAction1(new CommonAction1Params());
    }

    public function commonAction2(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        $worker->doCommonAction2(new CommonAction2Params());
    }

    public function specificAction1(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        if ($worker instanceof SpecificAction1AwareInterface) {
            $worker->doSpecificAction1(new SpecificAction1Params());
        } else {
            // do something
        }
    }

    public function specificAction2(string $workerId, WorkerFactory $factory)
    {
        $worker = $factory->createWorker($workerId);

        if ($worker instanceof SpecificAction2AwareInterface) {
            $worker->doSpecificAction1(new SpecificAction2Params());
        } else {
            // do something
        }
    }
}

But this code seems ugly I think. I'm not sure that using instanceof is a good idea especially because SpecificAction1AwareInterface and SpecificAction2AwareInterface aren't related to WorkerInterface at all.

So are there any design patterns suitable for my situation? Thank you in advance.

Upvotes: 1

Views: 81

Answers (0)

Related Questions