如何使用依赖注入？

Question

I'm new to OOP, so please be patient with me;)

Feel free to comment the code.

I have a class RegisterFormParameterHandler that extends FormParameterHandler. I'm using it to validate $_POST variables for register or login user. Class 'notice' is for error reporting and log.

I got the code working by passing object $notice as an argument of constructor in RegisterFormParameterHandler. Should I use static methods in a class notice and how?

class notice {

private $_notice = array();

public function get_notice(){
    return $this->_notice;
}

public function add($type, $message) {
    $this->_notice[$type][] = $message;
}
}

and:

abstract class FormParameterHandler {

protected $parameters;

public function __construct($associative_array) {

    $this->parameters = array();

    foreach($associative_array as $key => $value) {
        $this->{$key} = $value;
    }
}

public function __get($key) {
    $value = null;

    if(method_exists($this, "get_$key")) {
        $value = $this->{"get_$key"}();
    } else {
        $value = $this->parameters[$key];
    }

    return $value;
}

public function __set($key, $value) {
        $value = addslashes($value);
        $value = htmlentities($value);

    if(method_exists($this, "set_$key")) {
        $this->{"set_$key"}($value);
    } else {
        $this->parameters[$key] = $value;
    }
}

and:

class RegisterFormParameterHandler extends FormParameterHandler {

protected $notice;

public function __construct($form_parameters, $notice, $tok_id, $captcha) {
    parent::__construct($form_parameters);
    $this->notice = $notice;

    $args = func_get_args();

    foreach($form_parameters as $key=>$value) {
        $key = 'validate_'.$key;

        $this->$key($args);
    }
}

public function validate_something($args) {
    if(something === true) {
        $this->notice->add('error', 'Error message');
        }
    }
}

is this the right way how I'm passing in $arg in method validate_something or there is a way to do that in a constructor?

Class notice is instantiated with autoloader before the class RegisterFormParameterHandler.

  $notice = new notice();
  .....
  $reg = new RegisterFormParameterHandler($_POST, $notice, $tok_id, $captcha);

so the class notice allready contains some error messages and is used after this class is called.

Is there a better way to use class notice in class RegisterFormParameterHandler?

Answer 1

This is more of a code review question, but I'll try to answer it by introducing some small changes while walking through the code:

public function __construct($associative_array) 
{
    $this->parameters = array();

    foreach($associative_array as $key => $value) {
        $this->{$key} = $value;
    }
}

This is usually not necessary, because you can mimic properties using __get() and __set() which you already have an implementation for:

public function __construct($associative_array) 
{
    $this->parameters = array();
}

I would like to touch on your magic __set method:

public function __set($key, $value) 
{
    $value = addslashes($value);
    $value = htmlentities($value);

    if(method_exists($this, "set_$key")) {
        $this->{"set_$key"}($value);
    } else {
        $this->parameters[$key] = $value;
    }
}

Why the addslashes() and htmlentities()? Those shouldn't be there, because escaping is not a concern of the class.

Onto the constructor of the RegisterFormParameterHandler.

public function __construct($form_parameters, $notice, $tok_id, $captcha) 
{
    parent::__construct($form_parameters);
    $this->notice = $notice;

    $args = func_get_args();

    foreach($form_parameters as $key=>$value) {
        $key = 'validate_'.$key;

        $this->$key($args);
    }
}

First off, more than three parameters in the constructor and most of them are not required straight away if you introduce a separate validate() method.

Let's remove the constructor completely and write a validate() method:

final public function validate($notice, $tok_id, $captcha)
{
    foreach ($this->parameters as $key=>$value) {
        call_user_func_array(array($this, "validate_$key"), func_get_args());
    }
}

Now, the dependency on $notice and the other two arguments are local to only the validate() method. I'm using call_user_func_array() here to proxy the arguments to the other validate methods, so that you can get some IDE code insight goodness:

public function validate_something(notice $notice, $tok_id, $captcha) 
{
    if(something === true) {
        $notice->add('error', 'Error message');
    }
}