Listener/Observer Model in PHP

Question

I've taken a look at the SPL Listener/Observer model but found that it doesn't work when using static methods. So I wrote my own but it's very simply. Can anyone suggest ways to make it more like the Model standards and if there is anything obvious that I've missed?

<?php
namespace modular\core;

class Listeners{
    private static $listeners = array();
    private static $triggered = array();
    private static $broadcasts = array();

    public static function attach($event, $key, $callback){
        $config = \modular\core\ModuleLoader::getModuleConfig($key);
        $config['instance'] = get_class($callback[0]);
        $callback['config'] = $config;
        static::$listeners[$event][md5($event.$key)] = array('module' => $key, 'callback' => $callback);
    }

    public static function detach($event, $key, $callback){
        unset(static::$listeners[$event][md5($event.$key)]);
    }

    public static function listeners($event = ''){
        return ($event == '') ? static::$listeners : static::$listeners[$event];
    }

    public static function listenersRegistered(){
        return static::$listeners;
    }

    public static function listenersTriggered(){
        return static::$triggered;
    }

    public static function getBroadcasts(){
        return static::$broadcasts;
    }

    public static function broadcast($event){
        $rendered = \modular\core\Renderer::rendered();

        $caller = debug_backtrace();//true, 1 - check version and implement this
        array_shift($caller);

        static::$broadcasts[$event][] = $caller;

        if(isset(static::$listeners[$event])){
            if(!$rendered)
                ob_start();

            foreach(static::$listeners[$event] as $listener){
                $myEvent = (\modular\core\Router::routed() ? ($listener['module'] == \modular\core\ModuleLoader::getActiveModule()) : true);
                if($myEvent){
                    static::$triggered[$event] = $listener;
                    $test = \modular\core\ModuleLoader::runModule($listener);
                }
            }

            if(!$rendered)
                ob_end_clean();
        }
    }
}
?>

The code to add a listener:

public static function addListener($event, $callback){
        $modules = \modular\core\ModuleLoader::getModules();

        if(($key = array_search($callback[0], $modules)) !== false){
            \modular\core\Listeners::attach($event, $key, $callback);
        }
        else{
            //problem - object not found...
        }
    }

_::addListener('pre-render', array(&$this, 'init'));

Callback is a callback function set as an array to specify the instance

The MD5 is used to combine both the $event and $key as a $key (module) can register to multiple events and vice-versa.

Broadcast is purely checking if any output has already occurred, if not then block all output if so then allow output whilst it calls the module callback method.

Because it's a framework static methods are used as my preferred method to access a class from multiple classes without having to worry about creating variables of instances that I either have to remember globally or re-instantiating the class constantly.

Answer 1

Because it's a framework static methods are used as my preferred method to access a class from multiple classes without having to worry about creating variables of instances that I either have to remember globally or re-instantiating the class constantly.

I don't do php, but I'll give this one a try.

Being a framework shouldn't be a reason to prefer static methods everywhere.

The Observer Pattern can be depicted as follows:

I've taken a look at the SPL Listener/Observer model but found that it doesn't work when using static methods.

There's a reason for that. If I read the code correctly, then when you have a class called SubjectA, and register 3 observers statically, every single instance of SubjectA will have the 3 observers registered; this is unlikely to be the intended behavior (not to mention threading issues), and it's only happening because of the static stuff.¹

It looks like your Listeners class is intended to be used in object composition, like, you'll create a Car class that has a Listeners instance, and you'll register listeners for an Explode event. Then you might have a Dog class that has a[nother] Listeners instance, and you'll register listeners for a Bark event. If I assessed the situation correctly, that will give you a barking Car and an exploding Dog, on top of the expected events.

_{¹ that is, if static in php has the same meaning as static in c#, which I'm assuming here.}

Answer 2

I see several problems here:

• It's hard to understand your code because you have not documented any of the methods and their arguments. I have trouble understanding what's the purpose of $event, $key and $callback arguments. First off I though that $callback must be a the callback function, but from code it looks like it's an array of some sort. What should it contain?

• I don't understand why the md5() function is used. Why don't you just index by $key?

• The broadcast() seems to be doing some rendering and output buffering. Looks like too many responsibilities for a Listeners class that should be dealing only with event delegation.

• There are static references to several classes like \modular\core\ModuleLoader. Which means your Listeners class is very tightly coupled with these. It also doesn't help my understanding of your code, as I don't have access to these classes.

• The fact that you have a need for a static Listeners class feels like a design smell. Too many static methods usually means you aren't really doing proper object-oriented programming, but rather just using classes as containers for your functions. Instead you should consider implementing it as a simple class with normal methods and just instantiating it as a globally available singleton.

Answer 3

$broadcasts are just for debugging purposes, right?

My thoughts:

• The name Listeners sounds to me like it's an enhanced homogenous collection. Considering a core part is broadcasting I would suggest renaming. I believe Doctrine2 uses name EventManager, so maybe something along those lines

• As suggested I would consider moving away from static-ness. You can arguably keep your static method ::addListener() and have an instance of Listeners as a static property if you really do want to keep the static access.

  But in a well-designed application (where classes have reasonable number of dependencies) handling instances shouldn't be an issue. The class will be more flexible and easier to test (for that matter I wouldn't even recommend the class itself as singleton, having just one instance can be handled as noted above).

  Take a look at Misko Hevery's Guide: Writing Testable Code (while individual points are arguable the overall approach is something worth at least considering).

• As also suggested ::broadcast() probably does more than it should, but I don't have enough understanding of your application to make a suggestion.

• I would rename ::listeners() to ::getListeners(), for clarify and for consistency with ::getBroadcasts(). Same for other ::listeners##() methods. Somewhat nitpicky would be to suggest usage of null over an empty string to mark an empty value

• Don't know if it's only for clarity here, but you keep referencing classes from the same namespace as Listeners using their fully qualified name. You can remove all those \module\core\ as it's all in the same namespace. You may also search for use keyword, may come in handy one day.

• You do not have to re-instantiate instances! If you want more parts of application (say classes) to have access to a certain instance you pass the instance to them in some way (like constructor or setter injection) and assign it to a property. (don't worry about passing-by-value as in case of instances PHP actually just passes an object ID, so it is pretty much a reference).

• MD5 is not needed, PHP indexes are already being hashed. Some nice articles:

  • Understanding PHP's internal array implementation

  • Hash algorithm and collisions