phpstandardspsr-1php-fig

PSR-1: 2.3. Side Effects: variable inside config file

PSR-1 includes recommendation 2.3. Side Effects:

A file SHOULD declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it SHOULD execute logic with side effects, but SHOULD NOT do both.

Consider this example (my own) inside of a config.php file:

/**
 * Parsing the database URL.
 * DATABASE_URL is in the form:
 *  postgres://user:password@hostname:port/database
 * e.g.:
 *  postgres://u123:pabc@ec2.eu-west-1.compute.amazonaws.com:5432/dxyz
 */
$url = parse_url(getenv('DATABASE_URL'));
define('DB_HOST', $url['host']);
define('DB_NAME', substr($url['path'], 1)); // get rid of initial slash
define('DB_USER', $url['user']);
define('DB_PASSWORD', $url['pass']);

If I do this, I'm effectively not respecting the recommendation. phpcs will, rightfully, complain about it, because of the variable:

FILE: config.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 1 | WARNING | A file should declare new symbols (classes, functions, constants, etc.) and cause no other side
   |         | effects, or it should execute logic with side effects, but should not do both. The first symbol
   |         | is defined on line 17 and the first side effect is on line 162.
-----------------------------------------------------------------------------------------------------------------

An alternative would be this:

define('DB_HOST', parse_url(getenv('DATABASE_URL'))['host']);
define('DB_NAME', substr(parse_url(getenv('DATABASE_URL'))['path'], 1));
define('DB_USER', parse_url(getenv('DATABASE_URL'))['user']);
define('DB_PASSWORD', parse_url(getenv('DATABASE_URL'))['pass']);

No variable, no problem. But this is WET and hard to read.

I understand the recommendation is just that, and that it says "SHOULD", not "MUST". But this still bugs me… For one thing, anytime I check the file phpcs will complain about it, but report it just once per line, leaving the door open to adding more "side effects" which have no place in a config file.

I'm still new to this whole PSR thing.

Did I miss any clever way to get rid of the variable, while keeping things readable?

A corollary would be: how do serious projects, that insist on following recommendations to the letter, handle this?

Solution

  • 1. It's fine, don't sweat it

    You already mention it in your question, but this recommendation is a SHOULD and not a MUST. If this is the only PSR-1 issue in your entire project: good job!

    But your question was: how do other projects go about this?

    2. Move away from defines for configuration

    Global constants, when used incorrectly, are dependency magnets. They introduce coupling and make your code harder to digest. This Q&A is a very good read on why you should move away from them.

    Use dependency injection instead (yes, scalar configuration constants are also dependencies).

    3. Case study: Symfony

    Symfony-based projects use:

    For example, to configure a Database service in a Symfony project you'd create a YAML file that contains:

    services:
    My\Database\Factory: # <-- the class we are configuring
        arguments:
            $url: '%env(DATABASE_URL)' # <-- configure the $url constructor argument

    Symfony compiles this into PHP code, injecting the DATABASE_URL environment variable into the class that requires it.

    You would then parse DATABASE_URL in the constructor of the My\Database\Factory class, and use the result to construct your database class.

    Pros:

    Cons: