Handling Multiple Class Operations in a Factory

Question

I have the following Factory Method:

public function createErrorNotifier($verbose_output = false, $logging = false)
{
    // First we get the handler
    $errorHandler = $this->buildErrorHandler();

    $errorNotifier = $this->buildErrorNotifier();

    // We attach the notifier to the handler
    $errorHandler->setCallback(array($errorNotifier, "throwException"));

    // Return the Notifier
    return $errorNotifier;

}

protected function buildErrorHandler()
{
    return new ErrorHandler(error_reporting());
}

protected function buildErrorNotifier()
{
    return new ErrorNotifier();
}

Basically, $errorHandler is a class that, when it detects a PHP error, calls the
$errorNotifier->throwException() function.

The problem is that after the function is ran, and the class is set up, I don't have access to the ErrorHandler class, which means I can't turn off it/change properties/access methods, etc.

I'm wondering if the best method to do this would be to provide for a public accessor method to grab the errorHandler, something like:

public function buildErrorHandler()
{
    if($this->handler == null)
    {
        $this->handler = new ErrorHandler();
    }

    return $this->handler;
}

This method would allow the Factory to create a new instance of ErrorHandler, and would allow outside code to get access to the ErrorHandler. But I then run into the problem that if I want to go and create another ErrorNotifier, the first one will stop working, as I am reassigning the callback to the new object. This seems like it would be extremely bad practice, as it would be unexpected behavior.

I have a feeling that setting any sort of 'global' errorHandler would cause me to trip over this same problem, as the second time I got to call createErrorNotifier, the first one will not be called anymore.

Maybe a solution might be to instead give ErrorNotifier an instance of ErrorHandler, and then the ErrorNotifier can act as a proxy between the client and the ErrorHandler? Something like:

class ErrorNotifier{
     public function __construct(ErrorHandler $handler)
     {
          $this->errorHandler = $handler;
          $this->setCallback(array($this, "throwException"));
     }

     public function setCallback($callback)
     {
          $this->errorHandler->setCallback($callback);
     }
}

Another option might be to completely forget about the ErrorHandler, and rely on the client to tie the ErrorNotifier to some sort of handler (set_exception_handler(), ErrorhHandler, etc).

How would you handle something like this? I'm not at all against changing the design of the classes.

I grow very wary of just merging the two classes, as it would basically render the entire thing 'not-as-reusable'. If I separate the errorHandler functionality (calling a function when an error occurs) from the errorNotifier functionality (dealing with the error), I can then much more easily reuse both of them.

Answer 1

You could change createErrorNotifier() to return a standard object containing both the handler and notifier for the given instance. Then you won't need to worry about getters and mixing up a handler with a newer notifier. For example, try the following changes:

public function createErrorNotifier($verbose_output = false, $logging = false)
{
    // First we get the handler
    $errorHandler = $this->buildErrorHandler();

    $errorNotifier = $this->buildErrorNotifier();

    // We attach the notifier to the handler
    $errorHandler->setCallback(array($errorNotifier, "throwException"));

    // Return the Notifier
    return (object)array(
        'notifier' => $errorNotifier,
        'handler' => $errorHandler,
    );

}

Then when you call createErrorNotifier, you could access both the handler and notifier for a given instance:

$not1 = $obj->createErrorNotifier(...);
var_dump($not1->notifier, $not1->handler);

I hope that this helps!

Answer 2

You're probably best off having a proxy class or a singleton which would enable you to get at the error handler when you need it. Something like:

class ErrorHandler {
    private static $_instance = null;
    public static function getInstance()
    {
         if (null !== self::$_instance) {
              self::$_instance = new ErrorHandler;
         }
         return self::$_instance;
    }

    public static function setInstance(ErrorHandler $instance)
    {
         $oldInstance = self::$_instance;
         self::$_instance = $instance;
         return $oldInstance;
    }
}

This would allow you to always collect a single instance of ErrorHandler (via ErrorHandler::getInstance() ) and also globally change the active instance whenever you need to ( via ErrorHandler::setInstance() ).