How to simplify test with right pattern

Recently I stumbled upon some tests which were quite messy, and looked more or less like this:

Tested method:

   
    public function foo($mailer, $selectedPaymentMethod)
    {
        switch($selectedPaymentMethod) {
            case 0: {
                $mailer->caseOneMethod();
                break;
            }
            case 1: {
                $mailer->caseTwoMethod();
            }
        }`
    }

It’s of course simplified, but you’ll find similar stuff in your code if it’s old enough and you look closely.

The test method was mock mess:

    
public function test_foo_calls_CaseOneMethod_when_state_is_zero()
    {
        $object = new SwitchCase();
        $serviceStub = $this->getMock("stdClass", ['caseOneMethod', 'caseTwoMethod']);

        $serviceStub->expects($this->once())
            ->method('caseOneMethod');

        $object->foo($serviceStub, 0);
    }

This is only one case. To have it fully covered I should create method for state 1. And the add requirements that the other method is not called at all. Lot’s of work, and more coming if more cases appear (remember – this is payment method, and those are never enough for your client).

This is road to nowhere. If such simplified example seems not to be problematic – try for yourself add 5 cases. Then manage in some reasonable way list of methods which should and should not be called. How do you refactor? Extract $methodsWhichShouldNotBeCalled, use dataProviders? But there’s an easier way.

Enter polymorphism. It’s not just something you had to learn on your OOP course and what sophisticated programmers talk about. It’s also making programming simpler. And since writing test is also programming – it will make your tests simpler.

First I defined interface, and simple workers which represent different methods which has to be performed for different payment methods:

interface IMethods
{
    public function doYourJob();
}

class MethodOne
    implements IMethods
{
    public function doYourJob()
    {
        return __CLASS__ . " doing it's job";
    }
}

class MethodTwo
    implements IMethods
{

    public function doYourJob()
    {
        return __CLASS__ . " doing it's job";
    }

}

Now I move decision tree to new class which job is to return method depending on conditions:

 
class MethodFinder
{

    /**
     * @param int $selectedPaymentMethod
     * @return IMethods
     */
    public function findMethod($selectedPaymentMethod)
    {
        switch($selectedPaymentMethod) {
            case 0: {
                return new MethodOne();
            }
            case 1: {
                return new MethodTwo();
            }
        }
    }

}

You can think: nothing changed, I still have this damn switch case. But look at the test:

class MethodFinderTest
    extends \PHPUnit_Framework_TestCase
{

    /**
     * @dataProvider statesToMethodsMapping
     */
    public function test_methodFinder_finds_correct_method($paymentMethod, $expectedClass)
    {
        $methodFinder = new MethodFinder();

        $this->assertInstanceOf($expectedClass, $methodFinder->findMethod($paymentMethod));

    }

    public function statesToMethodsMapping()
    {
        return [
            [0, 'MethodOne'],
            [1, 'MethodTwo'],
        ];
    }
}

The difference is immense!

Now the test is about what method returns, not what is and what isn’t called internally. No mocking here, and simple array is easy to read – which paymentMethod means which method.

Original method also changes a lot:

class PolyExample
{

    public function foo($paymentMethod)
    {
        $methodFinder = new MethodFinder();
        $method = $methodFinder->findMethod($paymentMethod);
        $method->doYourJob();
    }

}

No branching. Of course – first line also needs some attention (it’s a implicit dependency of your class, so you would rather pass methodFinder as a param or use some kind of service locator to make it mockable and be able to test that “doJourJob” is called on whatever it returns. But that’s a detail and it depends on your architecture and other constraints.

As a closure I’ll tell you a secret: it’s a design pattern. I honestly don’t remember how it was exactly called. Some mixture of strategy and factory… I guess. To find out read about patterns and try to find the one that fits ;) This is your homework – leave a comment when you find out.

Also you may play with this code by yourself: https://github.com/pawlik/ScratchPad/tree/SwitchCaseVsFactory

Share Button

Leave a Reply

Your email address will not be published. Required fields are marked *