Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Created this little config class, any room for improvements? Think I have done an alright job but would like to know if any improovements could be made?

<?php
defined("SECURE") or exit('Please define SECURE keyword to continue.');
class config
{
    private $configValues;
    private $configFile;

    public function __construct()
    {
        $configValues = array();
    }

    public function loadConfiguration($configFile = "C:\Users\ashle\Miracle\config\config.ini")
    {
        if (!file_exists($configFile))
            loadDefault();

        $content = file($configFile);
        foreach ($content as $line)
            $this->setConfig(explode("=", $line)[0], explode("=", $line)[1]);
    }

    private function loadDefault()
    {
        /* DEFAULT CONFIG SETTINGS!!
           THIS SHOULD NOT BE THE CASE... */

        setConfig("database.host", "localhost");
        setConfig("database.username", "root");
        setConfig("database.password", "");
        setConfig("database.title", "mydb");
        setConfig("database.port", "3306");
    }

    private function setConfig($name, $value)
    {
        $this->configVlaues[$name] = $value;
    }

    public function getConfig($name)
    {
        return $this->configValues[$name];
    }
}
?>
share|improve this question

Things that just pop up in my mind when seeing the code (more of the architecture and robustness rather than language constructs):

1) Harcoding path to ini file

The path might have a meaning in your development environment, but it will mean nothing in a test or production environment. I would set it in an included file and use it from there. Something like:

$defaultConfigFilePath = "C:\Users\ashle\Miracle\config\config.ini";

in loadConfiguration $configFile = !isset($configFile) ? $defaultConfigFilePath : $configFile;

Hopefully, they will introduce a coalesce operator soon to simply write:

$configFile = $configFile ?? $defaultConfigFilePath;

2) Error handling

loadConfiguration function does not handle any incorrect formatting. Any line which is not in the format name=value will not be imported correctly.

You can explode into an $lineArray and check if elements are defined and exactly two.

3) A more powerful config reader

This is a good exercise, but ini files have a more complex format that seems to be handled by parse_ini_file function.

share|improve this answer
    
Thanks for your answer, one thing that popped into my head to ask was what is the speed on something like this like? I mean would it be faster to just make a file and havea variable for each config item? Also security.. is this more secure locating the config file outside the website folder? – Joe Dash Feb 20 at 13:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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