patternphpMinor
PHP Config Class
Viewed 0 times
phpconfigclass
Problem
I'm working on a config class for PHP, where I can easily bring in config options. Any improvements/suggestions are appreciated.
Example usage:
Example config file:
The class:
I should also add that this doesn't seem to work with multidimensional arrays - I'm working on this. If anyone has a solution, I'd appreciate it.
Example usage:
echo Config::get('database.host');
// Returns: localhostExample config file:
'localhost',
'username' => 'root',
'password' => '',
'database' => 'example_database'
);The class:
<?php
class Config {
/**
* All of the items from the config file that is loaded
*
* @var array
*/
public static $items = array();
/**
* Loads the config file specified and sets $items to the array
*
* @param string $filepath
* @return void
*/
public static function load($filepath)
{
static::$items = include('config/' . $filepath . '.php');
}
/**
* Searches the $items array and returns the item
*
* @param string $item
* @return string
*/
public static function get($key = null)
{
$input = explode('.', $key);
$filepath = $input[0];
unset($input[0]);
$key = implode('.', $input);
static::load($filepath);
if ( ! empty($key))
{
return static::$items[$key];
}
return static::$items;
}
}I should also add that this doesn't seem to work with multidimensional arrays - I'm working on this. If anyone has a solution, I'd appreciate it.
Solution
You can continue to use a class if you want, but I think making a config class is overkill. Just a plain config file with constants should be good enough. It has less overhead and less abstraction. As for loading a specific config file, you can use a helper function to simulate what you have here, or just manually include it. Not everything has to be in a class.
I think there's something wrong with your
I'm not sure I really agree with setting
You can set an array element to a variable and unset it all at the same time by using
It might be worth profiling that explode/implode task and comparing it to a substr version. Assuming the array version is faster, then I would suggest changing the first delimiter to something other than a period since that's the only one you care about. Maybe a forward slash or underscore instead? It would make needing to implode the array unnecessary.
PHP
Finally, I would think that the "default" return value for your getter would be NULL or FALSE. Why return the entire configuration array? This could be confusing, especially since you mentioned using multidimensional arrays. Speaking of which, what about this doesn't work for them? The config file? The getter? What exactly is happening, and what are you expecting? Besides the things I already mentioned, I don't see anything that would cause an issue.
Edit to clarify last comment:
Early check and return ensures less overhead and better legibility.
I don't condone the use of variable-variables, but its the only way I can foresee doing this. Unless you can put all of those arrays into another array, then you can access them as associative arrays.
echo Config::get( 'database.host' );
//VS
echo DB_HOST;
$config->load( 'config' );
//VS
config( 'config' );//helper function
//OR
include 'config/config.php';I think there's something wrong with your
load() method by the way. You are resetting the array each time you load a new config file. I believe it should look something like so.static::$items[] = include "config/$filepath.php";I'm not sure I really agree with setting
$items to static. I understand you aren't instantiating the class, but I think you should, and then just make this a normal property. Typically when I think of static properties, I think of properties that aren't supposed to change, but you are adding to it, which seems contradictory. Maybe I'm wrong here, but this just seems odd to me. Of course I really just dislike the static keyword altogether.You can set an array element to a variable and unset it all at the same time by using
array_shift(). For example.$filepath = array_shift( $input );It might be worth profiling that explode/implode task and comparing it to a substr version. Assuming the array version is faster, then I would suggest changing the first delimiter to something other than a period since that's the only one you care about. Maybe a forward slash or underscore instead? It would make needing to implode the array unnecessary.
$input = explode( '/', $key);
$filepath = array_shift( $input );
$key = strval( $input );PHP
empty() should really only be used when checking arrays. A string can be determined just by querying it. For example:if ( $key ) {//not empty
if( ! $key ) {//emptyFinally, I would think that the "default" return value for your getter would be NULL or FALSE. Why return the entire configuration array? This could be confusing, especially since you mentioned using multidimensional arrays. Speaking of which, what about this doesn't work for them? The config file? The getter? What exactly is happening, and what are you expecting? Besides the things I already mentioned, I don't see anything that would cause an issue.
Edit to clarify last comment:
Early check and return ensures less overhead and better legibility.
public static function get( $key = null ) {
if( ! $key ) {
return static::$items;
}
//etc...I don't condone the use of variable-variables, but its the only way I can foresee doing this. Unless you can put all of those arrays into another array, then you can access them as associative arrays.
$input = explode( '.', $key );
$filepath = array_shift( $input );
if( count( $input ) > 1 ) {
$array = array_shift( $input );
$key = $array[ strval( $input ) ];
}Code Snippets
echo Config::get( 'database.host' );
//VS
echo DB_HOST;
$config->load( 'config' );
//VS
config( 'config' );//helper function
//OR
include 'config/config.php';static::$items[] = include "config/$filepath.php";$filepath = array_shift( $input );$input = explode( '/', $key);
$filepath = array_shift( $input );
$key = strval( $input );if ( $key ) {//not empty
if( ! $key ) {//emptyContext
StackExchange Code Review Q#15348, answer score: 3
Revisions (0)
No revisions yet.