Krdytkyu

I am trying to create a config class identical to Laravel's but wanted to do it myself, to test my knowledge on the PHP language, can anyone help me improve it?

Each config file is located in the /app path and can be named whatever you want .php.

You can then get your config values by doing
Config::get('file.array_key')

So lets say I had a file called app.php and an array with a config key I wanted to get called 'author' or something I could simply do Config::get('app.author') I could even dive deeper and go for a key of a key by doing Config::get('app.author.email')

If I wanted to go even deeper, what would be a better way other than just doing an if statement for each index? checking the length etc?

Here is my code:

<?php



namespace PhpTestAppAppLibraryConfig;



class Config 

{

    static private $configFiles = ;



    public static function loadConfig() {

        $files = scandir(AP . '/app/config/');



        foreach(glob(AP . '/app/config/*.php') as $file) {

            self::$configFiles[explode('.', basename($file))[0]] = include(AP . '/app/config/' . basename($file));

        }

    }



    public function get($getString) {

        $parts = explode('.', $getString);

        $countDot = count($parts);



        if ($countDot == 2) {

            return array_key_exists($parts[0], self::$configFiles) ? 

            (array_key_exists($parts[1], self::$configFiles[$parts[0]]) ? 

            self::$configFiles[$parts[0]][$parts[1]] : '...') 

            : '';

        }

        else if ($countDot == 1) {

            return ''; // just a file?

        }

        else {

            return '';

        }

    }

}

Where is AP defined? Should loadConfig just get passed a directory path as parameter rather than relying on some global constant and hard-coded path?

I would consider using SplDirectoryIterator and SplFileInfo functionality to load your config files, as this should allow to do this in a much more robust way in comparison to glob(). You could (and probably should), verify that the config directory and all files are readable to the application and throw an exception to bail out of the process if you can't load config properly.

Right now you are kind of "soft" loading the config. include(), for example, will not halt execution if the file could not be included. I would think execution should halt with exception thrown so that your application cannot even run if the dependencies aren't met.

Shouldn't get() be static, since that is the only context in which it makes sense (there is no instantiation of this config).

$configFiles as property name seems odd. This is you actual configuration right?

I guess I am not a big fan of deriving some configuration "keys" from file name and some from the actual data structures in the file. This seems a bit obfuscated to me. Is it really that hard to put top level key in the file?

i.e. file like:

return [

    'toplevelkey' => [

        'secondLevel' => [

            'thirdLevel' => [

                ...

            ]

        ]

];

and in your loader:

self::config = array_merge(self::config, require $filepath);

This also could conceivably allow for configuration overrides from config in one file with config from other (this may or may not be desirable for you). You would likely need to manage file load order in some manner for this, something you may not be able to do effectively with just using glob().

Consider adding helper method with get() to decouple key retrieval logic into it's own (testable) method with appropriate exception triggering to make your class less fragile to bad input. After all, if some portion of your application is expecting some config to be available and it is not what is expected, you should be failing loudly, because your dependency is not being met.

That might look like:

public static function get($keyString) {

    $keys = explode('.', $keyString);

    return self::findConfig($keys, self::config);

}



protected static function findConfig(array $keys, array $haystack, $keyString = '') {

    $key = array_shift($keys);

    $keyString = ltrim($keyString . '.' . $key, '.');

    if (!isset($haystack[$key]) {

        throw new OutOfRangeException("Config key '{$keyString}' is missing");

    }

    if(count($keys) === 0) {

        return $haystack[$key];

    }

    if(!is_array($haystack[$key]) {

        throw new UnexpectedValueException("Expected array value at key '{$keyString}'");

    }

    return self::findConfig($keys, $haystack[$key], $keyString);

}

This approach to use recursion to find a key in your config also unbounds you from your current hard-coded limit of only having 3 key segments.

Consider providing better validation for parameters passed to public methods. For example, validate get() input is non-zero length string. Particularly with something as critical to application operation as configuration, you want to take reasonable measures to make sure the class and its data are set up in an appropriate state and that other areas in application are interacting with it in a proper manner.