At the moment, I use a variable called $config, which is a big associative array containing settings for the application like folder configuration, database connection, urls to match, and so on. This variable is defined in its own file system/config.php which gets included by the main file system/index.php.
Some system functions which are located in i.e. system/request.php make use of this global config variable through global $config;
Is this bad practice, and why? What would be a better way to handle global configuration?
IMHO, as long as a global variable is not simulatenously modified by different processes (i.e. it's read-only for all of them), it is OK to have it in the global scope.
A way to simulate a read-only variable is to encapsulate it in a global object with a private variable in it and a public method that would return that variable.
Example:
class Config {
private $conf = array(/*read from file?*/...);
function getconf() {
//AFAIK, any attempt to write into the returned array will make PHP's
//core to duplicate it, so the original config won't be modified
//(as long as $conf does not contain objects, which are always addressed
//by reference)
return $conf;
}
}
Pulling in variables from nowhere (global namespace) is bad because you don't know how they get their value and because every class or function can change them. So unless you use some sort of final, const, etc. to protect them, they will break the system on the long run and you'll spend hours finding out how they got their values.
For OOP an alternative solution is using a containers and dependency injection.
$container = new Container(array(
'x' => array(
'a' => 8,
'y' => array(
'b' => 123,
'z' => 456
)
)
));
$x = $container->getX();
$x->doSomething();
If there is strong coupling between the classes, because you don't want them to be something general, then you can do this:
class Container {
public function __construct(array $config){
$this->config = $config;
}
public function getX(){
return new X($this->$config['x']);
}
}
class X {
public function __construct(array $config){
$this->a = $config['a'];
$this->y = new Y($config['y']);
}
public function doSomething(){}
}
class Y {
public function __construct(array $config){
$this->b = $config['b'];
$this->z = new Z($config['z']);
}
}
class Z {
public function __construct($number){
$this->number = $number;
}
}
If these classes are loosely coupled, but you want to keep the hierarchy in the config (for example because you have multiple Y and Z instances in your container with different configs), then you can do this:
class Container {
public function __construct(array $config){
$this->config = $config;
}
public function getX(){
return new X($this->$config['x']['a'], $this->getXY());
}
public function getXY(){
return new Y($config['x']['y']['b'], $this->getXYZ());
}
public function getXYZ(){
return new Z($config['x']['y']['z']);
}
}
class X {
public function __construct($a, Y $y){
$this->a = $a;
$this->y = $y;
}
public function doSomething(){}
}
class Y {
public function __construct($b, Z $z){
$this->b = $b;
$this->z = $z
}
}
class Z {
public function __construct($number){
$this->number = $number;
}
}
Even if you develop without OOP you should send your dependencies as parameters instead of global variables.
function main($config){
$x = $config['x'];
$a = $x['a'];
$y = $x['y'];
doXSomething($a, $y);
}
function doXSomething($a, array $y){
$b = $y['b'];
$z = $y['z'];
$p = doY($b, $z);
// ...
}
function doY($b, $z){
$q = doZ($z);
// ...
}
function doZ($z){
// ...
}
$config = array(
'x' => array(
'a' => 8,
'y' => array(
'b' => 123,
'z' => 456
)
)
);
main($config);
You code should depend only on local variables, so you can be sure, that they are not changed from a different function when the app is running.
Another reason using global variables is bad is testability. If you write a test for a function, then you have to set a config variable. If it is deeply nested, then you do something like
$config = array('x' => array('y' => array('z' => 123)))`;
$result = doZ();
expect($result)->toBe(435);
Without global variables you can do
$result = doZ(123);
expect($result)->toBe(435);
Based on the file names I guess you had some sort of spaghetti code back then 5 years ago.