Replace constant() with defined()

[pippinsplugins]

While I really don't think there's anything wrong with using the constant() function, it's a bit cleaner and more standard to use defined().

if (@constant('BRACKETPRESS_DEBUG')) {

becomes

if ( defined('BRACKETPRESS_DEBUG')) {

[pippinsplugins]

This is in the main plugin file and also here: https://github.com/ntemple/bracketpress/blob/master/bracketpress.php#L699

[ntemple]

The code is correct as written.

The intent of the code is to check if BRACKETPRESS_DEBUG is defined, and if
it is defined, if it is truthy.

defined() only tells you if the constant is defined or not.

constant(), on the other hand, will tell you:
a) if it's defined, or NULL it it is not defined
b) what the value is if it is indeed defined (again, or NULL if undefined)
(it will also throw a warning if not defined, which we suppress with the @
sign: http://www.php.net/manual/en/language.operators.errorcontrol.php )

See:
http://us2.php.net/manual/en/function.constant.php

In the case it's defined but false, or in the case it's not defined at all,
the debug code will not be run without printing errors or warnings (the @
suppresses any warnings).

The alternative would be the much more verbose:

if (defined('BRACKETPRESS_DEBUG') && BRACKETPRESS_DEBUG != false) { }
which is still slightly incorrect because, if not defined,
BRACKETPRES_DEBUG becomes a string, which evaluates to true and subtly
incorrect.

Any specific reason you bring this up?

On Sun, Feb 9, 2014 at 8:28 PM, Pippin Williamson
notifications@github.comwrote:

This is in the main plugin file and also here:
https://github.com/ntemple/bracketpress/blob/master/bracketpress.php#L699

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-34600182
.

[pippinsplugins]

That makes sense. For me it was mostly for "standards" reasons, but I fully understand that's not always a good reason :)

Is there any reason that BRACKETPRESS_DEBUG would ever be defined as anything other than true? Just by accident?

[ntemple]

In a community project, I'd treat it as "user input".

TRUE, true, 'true', 1, YES, etc should all be ok, and no "valid" input
should break the code. i.e., commenting / uncommenting the line (which
breaks defined) or defining it as something unexpected. Esp. with debug
type stuff, it's really person-dependent. If you're super concerned, we
could add a note on the "accepted" way to enable / disable debugging in the
README.md. As bad as the front-end view logic is (esp. the javascript)
debug code is pretty low on the priority list.

On Mon, Feb 10, 2014 at 12:30 PM, Pippin Williamson <
notifications@github.com> wrote:

That makes sense. For me it was mostly for "standards" reasons, but I
fully understand that's not always a good reason :)

Is there any reason that BRACKETPRESS_DEBUG would ever be defined as
anything other than true? Just by accident?

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-34678543
.

[pippinsplugins]

Ha I wasn't "concerned" about it :)

Very good points.

On Mon, Feb 10, 2014 at 10:03 PM, Nick Temple notifications@github.comwrote:

In a community project, I'd treat it as "user input".

TRUE, true, 'true', 1, YES, etc should all be ok, and no "valid" input
should break the code. i.e., commenting / uncommenting the line (which
breaks defined) or defining it as something unexpected. Esp. with debug
type stuff, it's really person-dependent. If you're super concerned, we
could add a note on the "accepted" way to enable / disable debugging in the
README.md. As bad as the front-end view logic is (esp. the javascript)
debug code is pretty low on the priority list.

On Mon, Feb 10, 2014 at 12:30 PM, Pippin Williamson <
notifications@github.com> wrote:

That makes sense. For me it was mostly for "standards" reasons, but I
fully understand that's not always a good reason :)

Is there any reason that BRACKETPRESS_DEBUG would ever be defined as
anything other than true? Just by accident?

Reply to this email directly or view it on GitHub<
https://github.com/ntemple/bracketpress/issues/8#issuecomment-34678543>

.

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-34724738
.