XH 1.7: Use const keyword instead of define()

cmb · Post by **cmb** » Fri Feb 27, 2015 1:07 pm

Hello Community,

I like to suggest that we define all proper constants with the const keyword instead with the define() function. Even though there's not much of a difference, the former is better readable and likely to be better supported by IDEs, and the implied restrictions (simple scalar values up to PHP 5.5.0; constant expressions since PHP 5.6.0) are much more suitable to the concept of a constant.

However, there are issues with regard to our development tooling. PHP_CompatInfo 3.7.3 (most likely the last version of the 3.x series) reports const constants to require PHP 5.6.0, while PHP_Compatinfo 4 is still in beta phase, and reports these to require PHP 4.0.0 (while 5.3.0 would be appropriate). I file a bug reports against the latter behavior, so maybe this will be solved soon.

A bigger problem is PHPDocumentor 1.4.4, which does not recognize global const constants at all (class constants are recognized, though).

So I'm not sure what to do. What do you think?

svasti · Post by **svasti** » Fri Feb 27, 2015 5:46 pm

What's wrong with define? Coudn't find anything about php const... except for c++
If there are so many problems, why bother?

Holger · Post by **Holger** » Fri Feb 27, 2015 5:51 pm

cmb wrote:Even though there's not much of a difference, the former is better readable and likely to be better supported by IDEs

Ok, it's really better readable and - AFAIK - faster then define(). Beside this, my IDE has no problem with both.
But I think the differences are a bit more than "not much". Consider the following snippet:

Code: Select all

<?php

    $t = 1;
    
    if ($t === 1)
    {
        const FOO = 1;
    }

The code above will throw

Code: Select all

Parse error: syntax error, unexpected 'const' (T_CONST)

at least on my PHP 5.5.15 / Win. Not sure if that'll change with PHP6 or PHP7 -- but that's not the question for XH 1.7.
It's impossible to use const conditional but that's possible with define(). It seems to me that this is the biggest disadvantage.

EDIT: the manual brings some light to that issue:

Im Gegensatz zu Konstanten, welche mit define() ersellt werden, müssen über const definierte Konstanten im Top-Level Scope erstellt werden, weil sie zur Compile Zeit definiert werden. Das heißt, dass sie nicht innerhalb von Funktionen, Schleifen oder if Statements definiert werden können.

http://php.net/language.constants.syntax

Tata · Post by **Tata** » Fri Feb 27, 2015 7:20 pm

I am no coder. But I am teacher of math and concerning to this, I always use to get my children/students to analyze the problems and explore all values included. IOW, having a task, the solver shall first define any known values, any "semi-known" values (depending on unknown ones) and all values to be found.
These priciples would be very understandible in CMSimple_XH *.php files - all further used variables and definition on the very beginning. So one can see at first which variables, function, expressions etc. will be used.
To me then a structure would be optimal:

/* Definition - Declaration */
...

/* Functions - Procedures */
,,,

/* Specials */
...

cmb · Post by **cmb** » Fri Feb 27, 2015 8:22 pm

svasti wrote:What's wrong with define?

Holger wrote:It's impossible to use const conditional but that's possible with define(). It seems to me that this is the biggest disadvantage.

I didn't suggest to deprecate define(), but rather suggest to use const for proper constants. For me, one of the nice advantages of const is, that it can't be used conditinally. In my opinion that fits very nicely to mathematical or physical constants, which are non-varying numbers (such as Pi or the speed of light), i.e. they always have the same value, independent of other values. That's stronger than the notion that they can't be altered once being defined, and allows for yet better reasoning about algorithms.

Of course, we'd have to stick at least with several define()s, for instance for XH_ADM, or even CMSIMPLE_URL. However, since quite a while I feel that it has been suboptimal to define these as constants, anyway. But now they're there, I'm not going to suggest to change them in the near future. I think we should reconsider them for "2.0", though.

svasti wrote:If there are so many problems, why bother?

If there were no problems, I wouldn't have hesitated to propose a respective patch to be voted on in the second sprint. I still believe that const constants would be a Good Thing, but especially with regard to PHPDocumentor 1, it appears that it's best to postpone any decision.

Tata wrote:These priciples would be very understandible in CMSimple_XH *.php files - all further used variables and definition on the very beginning. So one can see at first which variables, function, expressions etc. will be used.

In my opinion this technique is very suitable for rather small scripts which are used instead of HTML files. However, in CMSimple(_XH) all request's are going through cmsimple/cms.php, so there's lot of functionality that would have to be contained in this script (the complete core and all plugins). It appears to me that an object oriented approach is more suitable here.

Holger · Post by **Holger** » Sat Feb 28, 2015 1:13 pm

cmb wrote:I didn't suggest to deprecate define(), but rather suggest to use const for proper constants.

Well, you've asked for opinions

. Therefore I want to document, that "there's not much of a difference" not describes the whole story. IMHO there's much of a difference

.

Anyway, I would suggest to use const instead of define() everywhere where possible, because we've set PHP 5.3 as a minimum requirement for 1.7. Const is better readable, faster and, as you wrote, closer to that a constant is meant.
But it seems not possible WRT the development tooling. If those issues are solved, we should use const. And IMO it's not too bad to stick with define() until then.
I think there's no need for a longer discussion...

BTW

cmb wrote:However, since quite a while I feel that it has been suboptimal to define these as constants, anyway.

Would be happy to hear your suggestions about alternatives for XH_ADM for example.

cmb · Post by **cmb** » Sat Feb 28, 2015 2:03 pm

Holger wrote:Well, you've asked for opinions .

Yes, and I'm glad to have gotten some feedback.

Holger wrote:I think there's no need for a longer discussion...

ACK.

Holger wrote:Would be happy to hear your suggestions about alternatives for XH_ADM for example.

Currently the setting of $adm happens in XH_Controller::handleLoginAndLogout(). It would be possible to set a private/protected static class property, and then to offer a public static class getter method (say, XH_Controller::isAdmin()). Before we had the chance to use private/protected state, we would have had to use some constant, but even then it might have been better to treat that constant as private and to offer a function to access it; then we could have replaced the constant anytime without the need to change the published API (and if we like to change the public API in the future, we could trigger a deprecation notice). As it's now, we only could document that XH_ADM would be deprecated -- something that's easily overlooked.

Holger · Post by **Holger** » Sat Feb 28, 2015 3:33 pm

cmb wrote:Currently the setting of $adm happens in XH_Controller::handleLoginAndLogout(). It would be possible to set a private/protected static class property, and then to offer a public static class getter method (say, XH_Controller::isAdmin()). Before we had the chance to use private/protected state, we would have had to use some constant, but even then it might have been better to treat that constant as private and to offer a function to access it; then we could have replaced the constant anytime without the need to change the published API (and if we like to change the public API in the future, we could trigger a deprecation notice). As it's now, we only could document that XH_ADM would be deprecated -- something that's easily overlooked.

Indeed, I could follow the advantages and share your opinion. Thanks for the detailed explanation

.
To be honest, I have trouble to follow all those improvements -- but I try to give my best

.

cmb · Post by **cmb** » Sat Feb 28, 2015 3:59 pm

Holger wrote:To be honest, I have trouble to follow all those improvements -- but I try to give my best .

You mean for time reasons? Well, I have to admit that I'm rushing through (mainly to regain lost ground), what is not optimal. I think, however, that anybody can (and should) vote "later", if it's too fast to follow, and actually only the items for the next sprint need immediate consideration. I hope you and others have still some time left to push forward your preferred ideas.

svasti · Post by **svasti** » Sat Feb 28, 2015 6:33 pm

Sorry, I have not been able to find const in the php online manual. Where do I have to look for it?

CMSimple_XH–Forum

XH 1.7: Use const keyword instead of define()

XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()

Re: XH 1.7: Use const keyword instead of define()