mintobit
mintobit

Reputation: 2383

Which is the best practice to access config inside a function?

<?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.

Upvotes: 4

Views: 2617

Answers (2)

PeeHaa
PeeHaa

Reputation: 72672

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.

Upvotes: 2

Sergey Eremin
Sergey Eremin

Reputation: 11080

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

Upvotes: 2

Related Questions