2
\$\begingroup\$

I have to update a class property with the method name and then call a method that executes some code based on this property. The code is the same in both methods, but I am not seeing a way to remove this code duplication. I am trying to avoid using a parameter.

public function register(){
    $this->operation = __FUNCTION__;
    $this->handleOperation();
}

public function unregister(){
    $this->operation = __FUNCTION__;
    $this->handleOperation();
}

Update 1

As Yoda suggested, here is a more complete code. It is a class to manipulate multiple functions for PHP autoloading. Now I am using parameters in handleOperation(). Example of use:

$Autoloader = new Autoloader();
$Autoloader->setFunctions(array('controllers', 'commons'));
$Autoloader->setModule('default');
$Autoloader->register();

Class code:

class Autoloader {

    private $functions_to_handle = array();
    private $module;
    private $function;
    private $paths = array();

    public function setFunctions($functions){
        $this->functions_to_handle = is_array($functions) ? $functions : array($functions);
    }

    public function setModule($module){
        $this->module = $module;
    }

    public function register(){
        $this->handleOperation(__FUNCTION__);
    }

    public function unregister(){
        $this->handleOperation(__FUNCTION__);
    }

    private function handleOperation($operation){
        $built_in_function = $this->mountBuiltInFunctionName($operation);

        foreach($this->functions_to_handle as $function){
            $built_in_function(array($this, $function));
        }
    }

    private function mountBuiltInFunctionName($operation){
        return 'spl_autoload_' . $operation;
    }

    private function controllers($classe) {
        $this->function = __FUNCTION__;

        $this->refreshPaths($classe);
        $this->loadClass();
    }

    private function commons($classe) {
        $this->function = __FUNCTION__;

        $this->refreshPaths($classe);
        $this->loadClass();
    }

    private function refreshPaths($classe){
        switch ($this->function) {

            case 'commons' :
                $this->paths = array(
                    '/usr/share/evokernel/core/common/' . $classe . '.php',
                );
            break;

            case 'controllers':
                $this->paths = array(
                    'core/controllers/' . $classe . '.php',
                    'modules/' . $this->module . '/controllers/' . $classe . '.php',
                    'shared/modules/' . $this->module . '/controllers/' . $classe . '.php'
                );
            break;

            default:
                throw new Exception('Invalid function: ' . $this->function);
        }
    }

    private function loadClass(){
        foreach ($this->paths as $path) {
            if (is_file($path)) {
                require_once($path);
                break;
            }
        }
    }
}
\$\endgroup\$
18
  • \$\begingroup\$ Why are you trying not to use a parameter ? \$\endgroup\$ – konijn Apr 25 '14 at 19:19
  • \$\begingroup\$ @konijn, I read recently "Clean Code" by Robert Martin, there he discourages to use parameters, so i am trying to follow this recommendation when possible. \$\endgroup\$ – Marcio Mazzucato Apr 25 '14 at 19:21
  • \$\begingroup\$ That sounds so wrong ;) \$\endgroup\$ – konijn Apr 25 '14 at 19:37
  • \$\begingroup\$ @konijn, I didn't agree completly with this, but i am trying to see the benefits. Do you have a suggestion with parameters? \$\endgroup\$ – Marcio Mazzucato Apr 25 '14 at 19:39
  • 2
    \$\begingroup\$ Too many parameters is a bad thing, but even in Uncle Bob's own "clean" examples, some functions have parameters. I think it is good to avoid using parameters that make the code less readable and understandable, but also advisable to use a parameter (or two, or even three) when the alternative is worse. \$\endgroup\$ – David K Apr 25 '14 at 19:41
2
\$\begingroup\$

Short version: Use a parameter!

public function handleOperation($action) {}
public function register() { $this->handleOperation("register"); }
public function unregister() { $this->handleOperation("unregister"); }

Slightly longer version: Don't be a smartass

You don't need to dynamically call spl_autoload_$function, it's pointless and unreadable. Just call spl_autoload_registerand spl_autoload_unregisterwhen you need it. It's unlikely for the function names to change.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.