Helper function to simplify checking for constants

[evansolomon]

Love 'em or hate 'em, WordPress uses lots of constants. As a result, this pattern is all over core and plugins, and occasionally themes:

if ( defined( 'CONSTANT_NAME' ) && CONSTANT_NAME )

Right now on trunk, it's used 57 times (excluding 2 in Akismet).

~/code/wptrunk $ ack "defined\( ?('|\")([^'\"]+)\1 ?\) \&\& \2" -h --ignore-dir=wp-content/plugins/ | wc -l
      57

How about a new function to make that verbose logic a little bit less repetitive.

function wp_constant( $constant ) {
	return defined( $constant ) && constant( $constant );
}

[helenyhou]

Saw it ​discussed in IRC in relation to unit testing. I'm sure it's come up other times, too.

[nacin]

If we are to do something like wp_constant() it woud probably be so constants are pluggable (for testing purposes). This patch conflates not defining a constant with defining it as something that evaluates to false. In core, we make that assumption often, but in a generic function, we may not be able to, which means we might be in a situation where we need multiple functions just to be able to test everything.

We've reduces the use of the defined && constant pattern a few times before, notably with WP_DEBUG, which is automatically defined as false if it is not defined before the bootstrap begins to load.

I think this is clever but I would like to see what some PHP frameworks use to work with constants (for the ones they don't avoid). It'd be nice to see what we can do to reduce the number of constants we have. Those 57 instances have 30 unique constants among them.

[evansolomon]

Replying to nacin:

This patch conflates not defining a constant with defining it as something that evaluates to false. In core, we make that assumption often, but in a generic function, we may not be able to, which means we might be in a situation where we need multiple functions just to be able to test everything.

It definitely assumes that the distinction is not important. I agree multiple functions would probably make more sense than just one.

We've reduces the use of the defined && constant pattern a few times before, notably with WP_DEBUG, which is automatically defined as false if it is not defined before the bootstrap begins to load.

It's been increasing for a while. The WP_DEBUG change seems to have been in 2.9 (r12207), which also had the biggest jump in this pattern. Obviously in that time, lots of new code and features have been added too, so it may well have increased slower than it could have.

2.8 12
2.9 28
3.0 39
3.1 40
3.2 41
3.3 43
3.4 52

[MikeSchinkel]

Rather than go the direction of creating an access function I'd propose WordPress slowly deprecate all uses of PHP constants in the WordPress ecosystem and use another approach.

One potential other approach would be to deprecate constants in favor of UPPERCASED properties on the $wp object (uppercased properties should not conflict with current or I assume otherwise future potential "non-constant" properties.) This approach would change WP_DEBUG to be $wp->DEBUG.

Further for this approach we could:

• Add a magic __get() method to class WP that would return false for any uppercased properties that have not been previously set and set the property as false to avoid future __get() method calls.
• Add a magic __set() method to class WP that would define a PHP constant if not previously defined and if $wp->LEGACY_CONSTANTS == true.
• A wp_const class defined in /wp-load.php to be used as a proxy for setting constants on $wp prior to the instantiation of WP and assignment to $wp that occurs in /wp-settings.php.

This approach would allow:

• Access without having to test for existence.
• Fast access to constants, i.e. it would not require the overhead of a function call in most cases.
• Gradual migration, first for core and then for plugins and themes as developers decide to change.
• Display of a "deprecated" message if $wp->WARN_CONSTANTS == true by testing get_defined_constants() in a 'shutdown' hook, or similar.

This approach makes sense because:

• The $wp variable is the global object in WordPress and real constants are in the same scope.
• It adds very little code and effectively no overhead compared to the current case.
• It is consistent with existing practices as practically every entry-level plugin and theme developer understands the use of "global wp;" and/or "$GLOBALS['wp'].

I've implemented a proof-of-concept and it seems to work extremely nicely, better than I could have hoped for, actually.

If I get positive feedback on this idea I'll post the P.O.C. as a new ticket and a patch.

[MikeSchinkel]

Given the proposal above here's what part of a future wp-config.php might look like:

$wp->WP_DEBUG = true;

$wp->SAVEQUERIES = true;

$wp->WP_SITEURL = 'http://www.example.com';
$wp->WP_HOST = $wp->WP_SITEURL

// ** MySQL settings - You can get this info from your web host ** //
/** The name of the database for WordPress */
$wp->DB_NAME = 'site_db';

/** MySQL database username */
$wp->DB_USER = 'site_user';

/** MySQL database password */
$wp->DB_PASSWORD = 'abcde12345';

/** MySQL hostname */
$wp->DB_HOST = 'localhost';

/** Database Charset to use in creating database tables. */
$wp->DB_CHARSET = 'utf8';

/** The Database Collate type. Don't change this if in doubt. */
$wp->DB_COLLATE = '';

[wonderboymusic]

Sounds great, until that all gets obliterated by: ​http://core.trac.wordpress.org/browser/trunk/wp-settings.php#L244

[rmccue]

Replying to MikeSchinkel:

One potential other approach would be to deprecate constants in favor of UPPERCASED properties on the $wp object (uppercased properties should not conflict with current or I assume otherwise future potential "non-constant" properties.) This approach would change WP_DEBUG to be $wp->DEBUG.

On the one hand, I think this might be be confusing, simply because it's a weird sort of syntax (to my eyes at least) to use $wp->ABC and that not being a proper constant. However, it is a cool idea, especially with the magic getter/setter. I also think it makes sense to have it under $wp.

Replying to nacin:

I think this is clever but I would like to see what some PHP frameworks use to work with constants (for the ones they don't avoid). It'd be nice to see what we can do to reduce the number of constants we have. Those 57 instances have 30 unique constants among them.

In most cases, they simply avoid them completely. For example, setting Symfony into debug mode is just another configuration option, not a constant. One way that this could be done in WP is to load the WP class before the configuration, allowing users to set e.g. $wp->debug = true in wp-config.php. This would be a fairly major change, I'd say.

That's essentially the same as Mike's changes above. However, I think the configuration file should stick to constants for most things. Allowing setting the properties in that manner would allow them to be set anywhere in the code without a complicated setter. Such a complicated setter would probably be a bad thing too, because I can't see how you could possibly code such a thing without causing huge problems.

[MikeSchinkel]

Replying to wonderboymusic:

Sounds great, until that all gets obliterated by: ​http://core.trac.wordpress.org/browser/trunk/wp-settings.php#L244

Why would they get obliterated? My proof of concept is working.

[MikeSchinkel]

Replying to rmccue:

I think the configuration file should stick to constants for most things. Allowing setting the properties in that manner would allow them to be set anywhere in the code without a complicated setter.

What is really wrong with allowing them to be set anywhere, in practice? After many years experience I'm still looking for a good reason that "constants" need to remain immutable.

Such a complicated setter would probably be a bad thing too, because I can't see how you could possibly code such a thing without causing huge problems.

Could you elaborate on the potential problems?

[MikeSchinkel]

Replying to wonderboymusic:

Sounds great, until that all gets obliterated by: ​http://core.trac.wordpress.org/browser/trunk/wp-settings.php#L244

After re-reading your comment I think I see what you are saying. My P.O.C. does the following to deal with the issue:

/* 
 * Early in /wp-settings.php
 */
$wp_const = $wp;
unset( $wp );  // Some functions like register_taxonomy() will use $wp if it exists.
...
$wp = new WP();
/* 
 * This immediately follows the preceding line that assigns new WP() to $wp.
 */
foreach( get_object_vars( $wp_const ) as $property_name => $property_value ) {
  $wp->$property_name = $property_value;
}
unset( $wp_const );

[rmccue]

Replying to MikeSchinkel:

What is really wrong with allowing them to be set anywhere, in practice? After many years experience I'm still looking for a good reason that "constants" need to remain immutable.

I completely agree, but I think if they're going to be uppercase, then they need to be only settable once so that they act like constants. That's why my personal preference would be to use lowercased properties just as normal properties.

Could you elaborate on the potential problems?

That was for if they could only be set in wp-config.php, where I can see problems with using includes in the config causing the complicated setter stuff to fail. I think that's a complicated path, but I personally would prefer eliminating the constants and constant-like behaviour (immutability) in favour of simply using properties (also renaming some such as $wp->db['host'] = 'localhost' etc.). This could be done completely in a backwards compatible manner, since we're inventing these from scratch, so as long as they check for the constants as well, it should be fine, e.g.

class WP {
    public $db = array( 'host' => '', /* ... */ );

    // ...

    public function set_from_constants() {
        defined('DB_HOST') && $this->db['host'] = DB_HOST;
    }

[rmccue]

Replying to MikeSchinkel:

After re-reading your comment I think I see what you are saying. My P.O.C. does the following to deal with the issue:

/* 
 * Early in /wp-settings.php
 */
$wp_const = $wp;
unset( $wp );  // Some functions like register_taxonomy() will use $wp if it exists.
...
$wp = new WP();
/* 
 * This immediately follows the preceding line that assigns new WP() to $wp.
 */
foreach( get_object_vars( $wp_const ) as $property_name => $property_value ) {
  $wp->$property_name = $property_value;
}
unset( $wp_const );

I really like this, but how do we ensure $wp is defined before wp-config.php? If we don't, there'll be warnings (at least) from PHP. I guess adding $wp = (object) array(); to the top of wp-config.php by default is probably the easiest way.

[MikeSchinkel]

Replying to rmccue:

I really like this, but how do we ensure $wp is defined before wp-config.php? If we don't, there'll be warnings (at least) from PHP. I guess adding $wp = (object) array(); to the top of wp-config.php by default is probably the easiest way.

This is the rest of the P.O.C. code. I have it placed in /wp-load.php just before wp-config.php is called:

class wp_const {
  var $LEGACY_CONSTANTS = true;
  function __set( $property_name, $value ) {
    if ( $this->LEGACY_CONSTANTS && ! defined( $property_name ) ) {
      define( $property_name, $value );
    }
    $this->$property_name = $value;
  }
  function __get( $property_name ) {
    if ( preg_match( '#^[A-Z_0-9]+$#', $property_name ) ) {
      $this->$property_name = false;
    }
    return $this->$property_name;
  }
}
$wp = new wp_const();

Actually, I also have added the var $LEGACY_CONSTANTS and two magic functions to the WP class. Too bad ​Traits are not supported in PHP 5.2...

[MikeSchinkel]

Replying to rmccue:

I completely agree, but I think if they're going to be uppercase, then they need to be only settable once so that they act like constants. That's why my personal preference would be to use lowercased properties just as normal properties.

This approach could not support that and also offer the fast access to the properties. I guess it could be extended to have "read-only constants" by storing those in an internal array and thus always using the magic methods for access but the tradeoffs would obviously be slower access. That might be okay for constants not used often.

But even so, I've only seen immutable constants causing trouble vs. helping (such as for testing); in contrast I haven't seen any real issues with get_option() values being mutable.

That was for if they could only be set in wp-config.php, where I can see problems with using includes in the config causing the complicated setter stuff to fail.

I think this approach bypasses those issues. Maybe, maybe not?

I personally would prefer eliminating the constants and constant-like behaviour (immutability) in favour of simply using properties (also renaming some such as $wp->db['host'] = 'localhost' etc.).

Agreed. Obviously. :)

[alex-ye]

It might be a stupid question , but You worried a lot of the constants count ?! Is it really bad ?! and You just suggestion to use class vars instead of ?! Yes it's a good to clean the code but sometimes we the constants is a good practice ...

[MikeSchinkel]

Replying to alex-ye:

It might be a stupid question , but You worried a lot of the constants count ?! Is it really bad ?! and You just suggestion to use class vars instead of ?!

This is not about comment count, it is about making access easier i.e. so that you don't first have to test for if (defined('...')), and immutable constants cause difficulty in scenarios such as unit testing and/or with some plugin use-cases.

[nacin]

In theory, I like this. Making them immutable is an interesting idea.

Has anyone ever taken a look at old bbPress? It has a $bb object where you can set option names/values. I wonder if we can learn from their implementation (including what not to do).

I am all for repurposing the WP class for more things when it makes sense.

[deepwater wells]

Are there reasons not to implement a configuration object for this explicit purpose? Doing so would keep WP as a single-responsibility object (basically a router) while also allowing for the use of magic methods, defaults/fallbacks, etc. on config items. It would also allow the config object to be instantiated independent of (before) WP.

The config object (WP_Config?) could be accessible via a WP property (once instantiated), like $wp->config->some_setting, and/or through functions like wp_config_get('some_setting'). I think this would be more future-proof and intuitive than accessing WP directly. Thoughts?

[jeremyfelt]

I'd like to figure this out and start moving forward.

The conversation on this ticket is great and highlights several very useable ideas. I don't think it will be too tough for us to determine a nice path to start on. Some of the following is re-hash, some is fodder for brainstorming.

I see a few immediate questions that we should answer.

1. If the getting of config data is handled somewhere else besides constants, should an attempt be made to make these immutable or are they immutable now only because we've used constants from the start?
2. If config is immutable, can we dump it properly during unit test tear down.
3. Should some constants remain outside of a common config object?

It seems there are a handful of possible approaches, here are some summed up:

1. A $config property inside WP storing configuration key/value pairs.
2. Mimic the current configuration constants as individual properties inside WP.
3. A completely separate WP_Config or other class to hold and manage configuration.
4. Provide methods to manipulate config data into $_ENV.

I took a quick peek at a few other libraries and from what my unfamiliar eye could tell...

• Laravel uses $_ENV to store configurations provided via array in various environment specific config files.
• CakePHP uses a Configure class and a protected static $_values with setters and getters. Allows for the storage of arbitrary configuration outside of core code as well. Limited use of configuration constants.
• CodeIgniter uses a CI_Config class with a public $config array.

I stopped there, more examples are definitely welcome.

An intriguing pattern seems to be environment specific configuration files that are loaded in to populate stored configuration data. However, we're pretty tied to the constants currently defined in wp-config.php. We'll likely need to do something that sets a base configuration based on those values and some or all of what wp_initial_constants() provides.

Specific use cases that happen to all be testing related:

• Manipulating WP_INSTALLING during unit tests. #31130.
• Bring tests for ms-files back in from the cold. #30256
• Switch between multiple configurations during multisite bootstrap testing. Without the likes of 27884.diff.

I made 31130.diff to explore the use of wp_defined() and wp_define() as a way to get around the WP_INSTALLING setting. That patch uses a global $wp_config, which I don't think is a good final answer.

[johnjamesjacoby]

Replying to nacin:

Has anyone ever taken a look at old bbPress? It has a $bb object where you can set option names/values. I wonder if we can learn from their implementation (including what not to do).

Came here to say exactly this.

Here's a ​Gist from Flox.io of what's currently required to provide immutable predefined configurations that don't require touching the DB. It's not overly complex, but we can be more intentional in this area ala bbPress or any of the others Jeremy mentioned above.

I generally like everything I see in this ticket. Nice work everyone.

[swissspidy]

Related: #37699. There are now also more helper functions for constants, like wp_doing_ajax() for example and perhaps even wp_disallow_file_mods() (#38673).

[flixos90]

I prefer more specific functions to access values from constants, like those @swissspidy mentioned above.

This allows furthermore for specific filters, which are more flexible than a constant (unit tests!) and could be used as an alternative. I think it makes more sense to iterate over time to introduce more such functions in a consistent manner than a very general wp_constant().

[schlessera]

Just as a quick note:

A generalized function like the above should not return false if the constant is not defined (a problem that @nacin had already pointed out), but rather let the consumer provide the fallback value to use in that case:

function wp_constant( $constant, $fallback = false ) {
    return defined( $constant ) ? constant( $constant ) : $fallback;
}

But apart from that detail, I also think that this is just a crutch that further obfuscates the underlying problem: WordPress Core needs a standardized system for its bootstrap configuration.

Most systems nowadays use a configuration container of some sorts, and allow for the bootstrapping configuration defaults to be overridden by environment variables, so that you have control over them at the server level without the need to modify files. Most implementations then let you use real environment variables and .env files interchangeably.

[jorbin]

With the addition of helpers like the ones @swissspidy mentioned that are specific rather than generic like what was proposed here, I think this should be closed as wontfix.