Observer Pattern in PHP - Correct Practice for Different Events

Question

I would like to know if my implementation of the Observer pattern for different events, such as 'userLoggedOut', 'userRegistered', 'userLoggedIn' are correct, below is my simplified code:

index.php

$login = new \Observer\Observable\Login;
$userData = $login->getData();

$login->attach( new \Observer\Observer\Email );

$login->notify( 'userLoggedIn' );
$login->notify( 'userLoggedOut', $userData );
$login->notify( 'userRegistered' );

Login.php

namespace Observer\Observable;
use Observer\Interfaces;

class Login implements Interfaces\Observable {

    private $observers = array();

    function attach( Interfaces\Observer $object ) {

        $this->observers[] = $object;

    }

    function getObservers() {

        return $this->observers;

    }

    function notify( $action, $data = null ) {

        foreach( $this->observers as $observer ) :

            if( method_exists( $observer, $action ) ) $observer->$action( $data );

        endforeach;

    }

}

Email.php

namespace Observer\Observer;
use Observer\Interfaces;

class Email implements Interfaces\Observer {

    function userLoggedIn( $data = null ) {

        echo "email user logged in";

    }

    function userLoggedOut( $data = null ) {

        echo "email user logged out";

    }

    function userRegistered( $data = null ) {

        echo "email user registered";

    }

}

My question is are there better ways of implementing the Observer pattern when dealing with multiple actions.

Answer 1

I don't think you're implementation is wrong, but it just doesn't sit right with me. Your Observable has to know the events handler functions and implement them (without an interface of some kind to guarantee they are there. To continue the way you're going I'd make sure anything that would be attached to Login should implement an interface that guarantees those action functions will be there. From there, Login doesn't have to learn anything about the observer, it just calls the functions.

Another approach I would take would be to specific the handler for event names, like so:

class Observable {
    protected static $event_names = array();
    protected $observers = array();

    function __construct() {
        foreach (static::$event_names as $event_name) {
            $this->observers[$event_name] = array();
        }
    }

    function register($event, $object, $handler) {
        if (array_key_exists($event, $this->observers)) {
            $this->observers[$event][] = array($object, $handler);
        } else {
            echo "Invalid event \"$event\"!";
        }
    }

    function trigger($event, $data = null) {
        foreach ($this->observers[$event] as $observer) {
            $observer[0]->$observer[1]($data);
        }
    }
}

class Login extends Observable {
    protected static $event_names = array("userLoggedIn", "userLoggedOut", "userRegistered");
}

And then you're observers would register for events like so:

class SomeListener {
    function __construct() {
        $login_instance->register("userLoggedIn", $this, "myLoggedInHandler");
    }

    function myLoggedInHandler($data = null) {
        echo "User Logged In.";
    }
}

That's just an opinion though, it's a different approach but then it only requires the knowledge of what events fire (which your method could be argued to do as well, but the Observable should simply just call the handler methods IMO).