5
\$\begingroup\$

I am creating a system for a client that loads information from an XML file and then parses the information where needed. For this, I have got this class:

<?php
class Config
{
    private static $init;
    private static $link;

    private static $db_hostname;
    private static $db_username;
    private static $db_password;
    private static $db_database;

    public static $ABSPATH;
    public static $GWPATH;
    public static $CONFIGPATH;
    public static $CONNECTIONPATH;

    public static $configfile;
    public static $connectionfile;

    private static $cfg;

    const CONFIGFILEPATH = "E:/projects/php/_site/_site/_include/Config/web.config.xml";

    /** Private to implement singleton pattern */
    private function __construct() {  }

    /** @return Config */
    public static function Init()
    {
        return static::$init = (
            null === static::$init ? new self() : static::$init    
        );
    }

    public function RequireFiles()
    {
        require_once static::$connectionfile;
        require_once static::$GWPATH . 'libraries/functions/user.loggedin.php';
    }

    public function LoadConfig()
    {
        static::$cfg = simplexml_load_file(self::CONFIGFILEPATH);

        $path = static::$cfg->paths;
        $database = static::$cfg->database;

        static::$ABSPATH = (string) $path->abs; 
        static::$GWPATH = (string) $path->gwpath;
        static::$CONFIGPATH = (string) $path->configpath;
        static::$configfile = (string) $path->configfile;
        static::$CONNECTIONPATH = (string) $path->connectionpath;
        static::$connectionfile = (string) $path->connectionfile;

        static::$db_hostname = (string) $database->hostname;
        static::$db_username = (string) $database->username;
        static::$db_password = (string) $database->password;
        static::$db_database = (string) $database->name;
    }

    public function GetLink()
    {
        if (empty(static::$link) || null === static::$link)
            $this->SetLink();
        return static::$link;
    }

    public function SetLink()
    {
        static::$link = 
            new Database($this->GetDBHostname(), $this->GetDBUsername(), $this->GetDBPassword(), $this->GetDBDatabase()) 
                or die("Failed to connect to the server. Error: " . static::$link->connect_error);
    }

            private function GetDBHostname()
    {
        return static::$db_hostname;
    }

    private function GetDBUsername()
    {
        return static::$db_username;
    }

    private function GetDBPassword()
    {
        return static::$db_password;
    }

    private function GetDBDatabase()
    {
        return static::$db_database;
    }

    public function GetConfigXML($do = false)
    {
        if ($do == true)
            return static::$cfg;
        return null;
    }
}

This is working for me, but I wanted to know if there is a better way to do this?

Info:
This is used in this way:

$config = Config::Init();
$config->LoadConfig();
$config->RequireFiles();
$link = $config->GetLink();
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$
  • There is no need to make anything static here. I would pass the config file name in the constructor, and make everything an instance variable.

  • Any other object that needs configuration needs this config object, rather than referencing it via the singleton pattern

  • Storing the database configuration, like hostname, username and password makes sense in a config class. Actually connecting to the database does not.

  • Instead of using the die(...) function, throw an exception. Really, this would be made obsolete by not connecting to the database in a config class, but whenever you decide to do this, throwing an exception is more appropriate. You communicate that an insurmountable problem has occurred that you cannot fix, but you give other code in the system the option of fixing it.

  • The method names are in PascalCase -- idiomatic for C#, but not for PHP. The PHP Coding Style Guide from PHP-FIG is a good reference. Use camelCase instead, so LoadConfig becomes loadConfig.

  • The RequireFiles method isn't really configuration and doesn't belong in a config class. A class or function that has a more global scope in the application should be loading those, or take advantage of autoloading.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.