Is this a good service locator, and is this service locator pattern(?) OK?

Question

I'm working in Zend framework 1 and was using Zend_Registry to store objects so that I could replace them in my controllers while unit testing:

$auth = Zend_Registry::get('Auth'); // set in Bootstrap.php

I wanted to store closures that could be executed when they were required (rather than all during Booostrtap even if they weren't used in that request), but Registry doesn't seem to do this. I guess prior to 5.3 closures weren't around?

So, I built my own service locator class. I thought to extend Zend_Registry but prefer to have something that can be used outside ZF1, plus it's pretty basic anyway:

<?php

class App_Services
{
    /**
    * @var array $container will store our services
    */
    private $container = array();

    /**
    * Returns the *Singleton* instance of this class.
    *
    * @staticvar Singleton $instance The *Singleton* instances of this class.
    *
    * @return Singleton The *Singleton* instance.
    */
    public static function getInstance()
    {
        static $instance = null;
        if (null === $instance) {
            $instance = new App_Services();
        }

        return $instance;
    }

    /**
    * Get a service from the container
    * 
    * @param string $name Index of the service in container
    * @return various Service
    */
    public function get($id)
    {
        $instance = self::getInstance();

        // if not set, return null
        if (! isset($instance->container[$id]))
            return null;

        // if a closure, call it and return the return value
        if ($instance->container[$id] instanceof \Closure) {
            $instance->container[$id] = $instance->container[$id]();
            return $instance->container[$id];
        }

        return $instance->container[$id];
    }

    /**
    * Set a service in the container
    * 
    * @param string $id Name (index) of the service in container
    * @param string $service Service to store
    */
    public function set($id, $service)
    {
        $instance = self::getInstance();

        $instance->container[$id] = $service;
    }
}

.. and on Bootstrap I can do:

$auth = App_Services::set('Auth', function() {
    return new Auth();
});

.. then within my controller actions I can do:

$auth = App_Service::get('Auth');

.. as before.

Anyway, is this a good implementation of a service locator? Also could either this or Zend_Registry used in this way, be regarded as a service locator? I'd like to understand the pattern better as I read service locator pattern is a good idea, then I read it's an anti pattern, but I think some times it depends on how patterns are implemtemted whch makes then good patterns or anti patterns. I'd appreciate any advice on this, thanks.

Answer 1

I would change few things:

1. remove singleton, make private static $container = [] anyway you're just using static methods to retrieve it.
2. consider throwing special exception when getting not set service.
3. re-consider automatic caching for services (it is possible you may not want it for every service)

Considering pattern/anti pattern discussion I've found mostly cited is this article: http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/ which is summarized:

Let's examine why this is so. In short, the problem with Service Locator is that it hides a class' dependencies, causing run-time errors instead of compile-time errors, as well as making the code more difficult to maintain because it becomes unclear when you would be introducing a breaking change.

So basically it doesn't concerns you - all your errors are run-time errors in PHP. (Or unitTests run time error if you are smart :D).

Anyway you can turn every pattern into it's anti-version when overusing it. Think each time if some kind of dependency injection (DI) or other Inversion of Controll (IoC) fits you better.