Tuesday, 1 October 2013

Why i wish to remove getServiceLocator from AbstractController

Heya good people!

This is the first post on this blog so bare with me! So a few days ago i started a discussion after a PR was made against the ZF2. Here

I especially like the remarks from from a few people
https://github.com/zendframework/zf2/issues/5168#issuecomment-25174848
https://github.com/zendframework/zf2/issues/5168#issuecomment-25232532
https://github.com/zendframework/zf2/issues/5168#issuecomment-25205969

But none of these explicitly tell us why having the ServiceLocatorAwareInterface is an anti-pattern.
So here are my five cents to the discussion.

1, Using the service locator directly within the controller it's really easy to request new services. Which means it's very easy to lose track of the single responsibility principle (SRP) your controller should have.

2, Testing becomes more complicated because you will need to mock the service locator making your tests more fragile. Below you will find the difference between injecting dependencies and use the service locator

First example is using "proper" injection
class ExampleController extends AbstractActionController
{
    public function __construct(FooServiceInterface $fooService, BarServiceInterface $barService)
    {
        $this->fooService = $fooService;
        $this->barService = $barService;
    }

    public function testAction()
    {
        return $this->fooService->foo();
    }

    public function test2Action()
    {
        return $this->barService->bar();
    }
}
And here comes the test
class ExampleControllerTest
{
    protected function setUp()
    { 
        $this->fooService = $this->getMock('FooServiceInterface');
        $this->barService = $this->getMock('BarServiceInterface');

        $this->controller = new ExampleController($this->fooService, $this->barService);
    }
}
Second example is using the service locator anti-pattern
class ExampleController extends AbstractActionController
{
    public function testAction()
    {
        return $this->getServiceLocator()->get('fooService')->foo();
    }

    public function test2Action()
    {
        return $this->getServiceLocator()->get('barService')->bar();
    }
}
And it's test case
class ExampleControllerTest
{
    protected function setUp()
    { 
        $this->fooService = $this->getMock('FooServiceInterface');
        $this->barService = $this->getMock('BarServiceInterface');

        $sm = new ServiceManager();
        $sm->setService('fooService', $this->fooService);
        $sm->setService('barService', $this->barService);

        $this->controller = new ExampleController();
        $this->controller->setServiceLocator($sm);    
    }
}

Note
Both of the setup for our controllers are really quick and dirty and are far from perfect examples of how to properly test a controller.

Conclusion
As you noticed in the second example we also need to setup the ServiceManager and it's not a perfect setup. One could argue that i should modify the ServiceManager that is created by running the bootstrap on the Module. But that means we are doing more then we should be doing in the setUp method.

3, If your lazy like me you love the autocomplete of your favorite IDE but the docblock return param is mixed so your IDE won't be playing nice with you!

Well...... I could just add an annotation

/** @var FooServiceInterface $service */
$service = $this->getServiceLocator()->get('FooService');
return $service->foo();


So my final conclusion
We use the service locator anti-pattern for it's convenience but in the end we still end up writing the same amount of code that is harder to test, less readable and more confusing for new developers that join the project.