4

I have read almost all question I have found on StackOverflow on this topic, but could not find a straight answer.

Here is my code:

Application class

<?php
    class Application extends Settings {
        public function __construct($env, $cacheDir, $configFile) {
            self::$_env = $env;
            self::$_cacheDir = $cacheDir;
            self::$_config = $this->loadConfig($configFile) // reads configs from xml file into Config object
        }

        // other methods
    }
?>

Settings class:

<?php
class Settings {
    protected static $_env = null;
    protected static $_cacheDir = null;
    protected static $_config = null;

    public static function getEnv() {
        return self::$_env;
    }

    public static function getCacheDir() {
        return self::$_cacheDir;
    }

    public static function getConfig() {
        return self::$_config;
    }
}
?>

I access settings from anywhere in my code like this:

<?php
var_dump(Settings::getEnv());
?>

I want to access Settings form many different places. All values can be set only once and cannot be overwritten (so registry with __set methods do not work, because I can set any value from any place in any stage of application process)

Questions:

Is it good practice to store global settings like this. What downsides of this method? Maybe there's a much better way to do this?

Thank you for your answers

Tomas
  • 333
  • 1
  • 15
  • 3
    You're effetively global state with a the class, introducing a not exactly a singleton, but kind of like it. It has its uses, and its pros and cons. Me, I'm more a fan of dependancy injection, as these kinds of constructs are hard to write tests for. – Wrikken Jul 27 '11 at 17:25
  • @Wrikken I totaly agree with you. But I dont see the way to rewrite my code without singleton – Tomas Jul 27 '11 at 18:28

3 Answers3

4

Like Wrikken pointed out in the comment to your question, you are introducing Global State to your application. Quoting Martin Fowler on Global State (PoEAA, pg. 482f):

Remember that any global data is always guilty until proven innocent.

which in a nutshell means: avoid it. I leave it up to you to research on that topic though because it's out of scope for this question to discuss it in detail.

Now, for a better alternative

Let's assume you route all traffic to an index.php. You could then simply bootstrap/build all the components you need to fulfill the request inside that file. For instance, like this:

spl_autoload_register(
    function($className) {
        static $classMap = array(
            'request' => '/path/from/here/to/Request.php',
             … more mapping
        );
        require __DIR__ . $classMap[strtolower($className)];
    }
);

$config  = parse_ini_file(__DIR__ . '/path/from/here/to/config.ini');
foreach($config['env'] as $key => $val) {
    ini_set($key, $val);
}

$router = new Router;
$router->registerActionForRoute(
    '/product/list', 
    function($request, $response) use ($config) {
        return new ProductListAction(
            $request, $response
            new ProductMapper(
                new ProductGateway(
                    new MySqli($config['db']['host'], …),
                    new Cache($config['cache'], …)
                ),
                new ProductBuilder;
            )
        );
    }
);
$router->registerActionForRoute(…);
$router->execute(new Request($_GET, $_POST, $_SERVER), new Response);

Granted, you rather want to include the autoloader from a separate file (because you want to autogenerate it with something like https://github.com/theseer/Autoload). And of course you could replace the closures in the Router with Builder or Factory patterns. I just used the simplest thing possible. It's (hopefully) easier to understand this way. You can check http://silex-project.org/ for a micro-framework using a more sophisticated but similar approach.

The main benefit of this approach is that every component will get what it needs right from the start through Dependecy Injection. This will make it easier to unit-test your code because its so much easier to mock dependencies and achieve test-isolation.

Another benefit is that you keep construction graph and collaborator graph separate, so you dont mix up those responsibility (like you would with a Singleton or otherwise putting a new keyword into classes that are supposed to be Information Experts.

Community
  • 1
  • 1
Gordon
  • 312,688
  • 75
  • 539
  • 559
1

Can you post some more code? just to show how are you accessing those settings.

anyway, you could create a Boostrap class. This bootstrap class will do anything necessary to have working environment for your application (thus, moving out the bootstrapping code from the application and settings, to this class).

it can also instantiate a Settings object, which should be a singleton.

in the Settings object, you can use magic methods (__call, __get) to access the different settings, like Settings::getSettings()->getConfigDirectory(). This magic method will strip the "get" word from the call and try to give a resource with the given name (in this case, a setting named "ConfigDirectory").

This is similar to what Zend Framework does in their Zend_Application, Zend_Bootstrap, and Zend_Config classes, you might want to check them out to get some ideas.

as a side note, i don't see (conceptually speaking) why an application should extend settings. An application should have some settings, but that's quite different from extending them.

marcelog
  • 7,062
  • 1
  • 33
  • 46
  • 1
    There is no reason whatsoever for the Settings class to be a Singleton, nor is there any reason why he should litter that class with magic methods. If you really think this is how it should be done please elaborate your design decisions. – Gordon Jul 27 '11 at 18:12
  • please state your own solution then – marcelog Jul 27 '11 at 18:13
  • 1
    @Gordon: I completely agree, Singletons are bad, period. What if I wanted to have two different configurations at the same time, I couldn't have that with a singleton as there can only be one object. As for the magic `__get` and `__set` methods. They can be useful in a config class, but I would keep them simple. – MitMaro Jul 27 '11 at 18:43
  • singletons bad? 2 different configurations at the same time? never heard anything like it :) i can discuss, however, the use of magic methods (which they dont "litter" anything at all). but the first two statements... i dont see any valid arguments for you two to justify them. – marcelog Jul 27 '11 at 18:52
  • Here is some for Singletons: http://stackoverflow.com/questions/4595964/who-needs-singletons/4596323#4596323. Make sure you follow the links in that answer, especially the last one that links to the Clean Code talks. – Gordon Jul 27 '11 at 19:07
  • i agree about DI, i did not suggested in this case because it requires additional code/frameworks (ding, etc) that can do the IoC. i suggestes a singleton because the question said that the settings should be accessed from other code (which i dont approve, since the Application should be container, and that was not specified in the question). so the answer is strongly related to how the question was asked and the code posted. i wouldn't say singletons are always good, and i wouldnt say singletons are always bad either. it depends on what has to be done and the tools available to do it – marcelog Jul 27 '11 at 19:54
1

Your Application class should not extend Settings as there is no relationship between the two classes. Instead you should use dependency injection to include the settings into the Application class. There is an example of this below and I recommend reading up on dependency injection.

class Settings {
    // public to simplify example, you can add setters and getters
    public $_env = null;
    public $_cacheDir = null;
    public $_config = null;
}

class Application {
    protected $config;

    public function setConfig($config) {
        $this->config = $config;
    }

}



$app = new Application();

$config = new Settings();

$config->_env = 'dev';
$config->_cacheDir = '/my/dir';
$config->_config = array(/* Config here */);

$app->setConfig($config);

As mentioned by marcelog in another answer you could use a bootstrap class to handle the injection of the config, as well as other objects, into your Application class.

A basic example of a bootstrap class:

class Bootstrap {

    protected $application;

    public function __construct(Application $app) {
        $this->application = $app;
    }

    // connivence method
    public function init() {
        $this->initSettings();
    }

    public function initSettings() {
        $settings = new Settings();
        $settings->_env = 'dev';
        $settings->_cacheDir = '/my/dir';

        $config = array(); // load config from file here
        $settings->_config = config;
        $this->application->setSettings($settings);
    }

    // other init methods
}

$app = new Application();

$bootstrap = new Bootstrap($app);

$bootstrap->init();

These are very basic examples and there is nothing stopping you from writing magic getters and setters, having the bootstrap call any method that begins with init, etc...

MitMaro
  • 5,607
  • 6
  • 28
  • 52
  • And how I should access to these settings from other files of my app? Should I pass application class as a referece to every class that needs some settings? – Tomas Jul 27 '11 at 18:44
  • That would highly depend on your app layout and the classes where you need the settings object, in general there should only be a handful of places where you would need to inject the settings object. Give me some examples of what other classes need the settings information and I will try my best to help. :) – MitMaro Jul 27 '11 at 18:47