CONSTANT check

[♥raiwa]

In some older contributions I find the following check if a constant is true:

if (CONSTANT) {
...
}

Is this still useable/allowed in PHP 7.3 or should it better be replaced with:

if ( defined('CONSTANT') && CONSTANT === true ) {
......
}

 

[♥ecartz]

So long as I've been using PHP (since PHP4, circa 2003), those two pieces of code would never have been equivalent.  The first block has always been the equivalent of

if (!defined('CONSTANT') || CONSTANT != false) {

Because if not defined, it is interpreted as a bareword string, which is always truthy (evaluated as true).  I.e. it can only be false if it is defined as something that evaluates as false.  So if the constant's definition is missing, that code would interpret it as being true (which sounds undesirable).  In general, your second form is preferable both now and then. 

Note that bareword strings are to be deprecated in PHP 8, so you should probably avoid the syntax for that reason alone.  https://wiki.php.net/rfc/deprecate-bareword-strings

The question of whether to use == or === is trickier.  You might be better off with == in some cases, as that basically says that if the definition is messed up but there, include it.  Or you may prefer to default to off.  Or even

if (defined('CONSTANT') && CONSTANT) {

So long as you understand that this is generous in what it counts as true.  Or

if (defined('CONSTANT') && tep_not_null(CONSTANT)) {

The last is probably the most osCommerce way of doing it, presuming no one has removed or renamed tep_not_null while I wasn't looking.  It makes the most sense in situations where the constant is being used for something else, not just to hold a Boolean value.  That seems most consistent with the original usage.  I.e. rather than holding a Boolean value, I would expect the variable to hold an array, string, or object.  But of course you didn't include examples with context, so we can't say if that's how it was used in those examples.  Variants of

if (defined('CONSTANT') && is_array(CONSTANT) && !empty(CONSTANT)) {

might be even better in those cases.  Because then you don't have the problem of the constant being defined as a string, evaluated as not null (in string terms), and then have the code attempt to use it as an array.