4
<?php
define('ABSPATH', dirname(__FILE__)); //Absolute path to index

/*
* Method 1
* Dependency Injection
*/
class Config{

    private $_config = NULL;
    private $_filepath = NULL;

    public function __construct($filepath){
        $this->_filepath = $filepath;
        $this->load();
    }

    private function load(){
        if ($this->_config === NULL){
            if (!file_exists($this->_filepath)){
                throw new Exception('Configuration file not found');
            }else{
                $this->_config = parse_ini_file($this->_filepath);
            }
        }
    }

    public function get($key){
        if ($this->_config === NULL){
            throw new Exception('Configuration file is not loaded');
        }
        if (isset($this->_config[$key])){
            return $this->_config[$key];
        }else{
            throw new Exception('Variable ' . $key . ' does not exist in configuration file');
        }
    }
}

function getLost($where, $why, $who){
    //do smth
}

try{
    $config = new Config(ABSPATH . '/app/config.ini');
    getLost('here', 'because', $config->get('who'));    
}catch(Exception $e){
    echo $e->getMessage();
}
?>

<?php
/*
* Method 2
* Config is accessed via static class
*/

class Config{

    private static $_config = NULL;
    private static $_filepath = NULL;

    public static function load($filepath){
        if (self::$_config === NULL){
            self::$_filepath = $filepath;
            if (!file_exists(self::$_filepath)){
                throw new Exception('Configuration file not found');
            }else{
                self::$_config = parse_ini_file(self::$_filepath);
            }
        }
    }

    public static function get($key){
        if (self::$_config !== NULL){
            throw new Exception('Configuration file is not loaded');
        }
        if (isset(self::$_config[$key])){
            return self::$_config[$key];
        }else{
            throw new Exception('Variable ' . $key . ' does not exist in configuration file');
        }
    }
}

function getLost($where, $why){
    $who = Config::get('who');
}

try{
    Config::load(ABSPATH . '/app/config.ini');
    getLost('here', 'because');    
}catch(Exception $e){
    echo $e->getMessage();
}
?>

<?php
/**
* Method 3
* Config variable needed is passed as function parameter
*/
$config = parse_ini_file(ABSPATH . '/app/config.ini');

function getLost($where, $why, $who){
    //do smth
}

getLost('here', 'because', $config['who']);
?>

<?php
/*
* Mathod 4
* Config is accessed inside a function via global
*/
$config = parse_ini_file(ABSPATH . '/app/config.ini');

function getLost($where, $why){
    global $config;
    $who = $config['who'];
}

getLost('here', 'because');
?>

Which of these variants is the best practice solution? If none, please provide your variant.

2 Answers 2

2

I would go for variant 1 (dependency injection).

Variant 2 uses static methods which are as already stated just enother way of global methods. And we all know that is bad right? right?

Variant 3 isn't me favorite because it's not OOP enough ;-), but serious: what if you (or the person using your code) wants to change the format of the config file?

Variant 4: global...

So basically my issues with other options are: testability, tight coupling, global.

So variant 1 it is. I would also create an interface for the config class so you may add a different class for your config stuff at a later point. E.g. you (or someone else for that matter) wants to use XML files for config.

Another thing I would change is private stuff. If someone wants to extend the class he/she wouldn't have access to the variables this way. My rule of thumb (not sure if everybody agrees with this though) is only make stuff private if you want people to have access to it (e.g. it will change at some point). The danger of using private is that people will get around it by hacks to do what they want.

Read more about SOLID and see the Google clean code talks for more info.

Just my 2 cents.

1
  • 2
    I wouldn't necessarily agree with the blanket statement that one should immediately do away with private. Not all algorithms should automatically be subject to modification by child classes. Composition is generally preferable to inheritance. Judicious protected usage can go a long way to keeping the black box "black." That said, +1 :)
    – user895378
    Jul 17, 2012 at 17:42
2

I'd say first case is better if you replace $config with $onlyTheStuffThatMattersToThatFunction

1
  • 1
    Doesn't answer the question but I have to +1 because you're so right and this is the only sensible answer at this point :)
    – user895378
    Jul 17, 2012 at 5:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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