Remove PHP 4 Constructors

[valioz]

PHP 7 will emit E_DEPRECATED whenever a PHP 4 constructor is defined.

[EyeAndTea]

(Please note that we are not from the project collaborators or maintainers)

Thank you for your submission. We are of the opinion of keeping the backward compatibility as much as possible. This is one of the advantages of ADODB, atleast to us. This is also the reason why we continue using 'var' instead of enforced access modifiers such as "public", or "private" but include them in the comments.

If we may advice, it is too early to worry about PHP 7. However, we suggest that you add the new __construct functions and have them call the PHP 4 constructors. Make sure a __construct function is defined first in the file relative to the php 4 construct function.

[mnewnham]

Your point is well taken, While we strive to maintain backward compatibility, we must also accept that at a point we need to properly support newer versions. There is currently a discussion about creating a version 6 branch. That would be an appropriate place to eliminate version 4 compatibly. The verdict on this discussion will happen in 3-4 weeks time, until then, this pull request will remain outstanding

[valioz]

PHP4 is not supported by almost 7 years (7 Aug 2008). ADOdb library switched to PHP5 year earlier (http://phplens.com/phpeverywhere/?q=node/view/240). In some of the files PHP4 constructors has been removed (see for example adodb-active-record.inc.php changes in 580e33a).
Do you have a real life situation in which you need to use ADOdb library for PHP5 in PHP4 environment and you can not use PHP4 version of ADOdb (branch::adodb4)?

I agree, we have time, but It is not too early for PHP7, release is planned for November 2015.
I аm glad to hear that there are plans for a new version of the ADOdb library.

[Mike-Benoit]

Are recent versions of ADODB even actively tested on PHP4?

I'm all for maintaining backwards compatibility within reason, but I'm pretty sure recent versions of ADODB are riddled with land mines when run on PHP4, but I definitely agree its time to let go of PHP4. People still using PHP4 are unlikely to be using the latest version of ADODB anyways.

PHP7 and HHVM both offer massive performance improvements and don't play nice with PHP4 code. For this reason alone I expect to see a fairly high adoption rates and it would be nice to have ADODB ready to go when PHP7 is released at least.

[mnewnham]

All the points you have made are absolutely valid, as are those made by @CreatrixTeam. Yes it's true that php4 is obsolete and finding someone still using adodb 5 with php4 would be like finding a needle in a haystack and it's likely there would be no harm in merging this, but conversely there is no harm in holding it until an appropriate new master is made available.

As a matter of interest, if you are interested in eliminating PHP4 and ensuring PHP7 compatibilty , would you consider doing a code review to help us identify these issues, for example in drivers/adodb-db2.inc-php the constructor you changed looks like this:

function __construct()
 {
        $this->_haserrorfunctions = ADODB_PHPVER >= 0x4050;
 }

So _haserrorfunctions will always be true, because the PHP version will always be greater than 4, and we should eliminate all references to it. There are more than 30 occurrences in the system of this methodology that ought to be eliminated.

[EyeAndTea]

Thank you for all participators. I am the main IT here, and govern everything by strict philosophy and abstraction. A full explanation to the why of our position is tedious and long. I shall try to keep it as simple as possible. Please note that this has not been written for entertainment, but for the reader to pay careful attention to the simple writ knowing it was factored from complex.

Backward compatibility is not the goal itself. It is a common result to two principles, reduction and harmony. Harmony is a corollary to reduction. Question, what was achieved by this commit? The answer is nothing if we follow strict logic. Practically, something was achieved, but there are types to practical. If the practical is not the result of logic, it is the result of human artificial. Human artificial is the result of self, and hence dies with the human. In the end no vote can stop time, no protest can create. The human artificial, with threat we can make the good costly, and the bad profitable. In simple terms, if we follow logic, not the human artificial, no value was added by this commit. Before the commit, the code had constructors, and after the commit the code had constructors, and no more. The human artificial dictates that the after the commit works with newer PHP.

But harmony. That newer code causes a lose of information. Mostly all coded so far was coded with effort and deliberation toward a particular goal differing to the result of the new commit. In simple terms, was written for PHP 4, and this for PHP 7. Question, are you able to track all code written specifically for PHP 4? Answer, is a no, feasibility wise. It is not just a matter of syntax, the way a constructor is defined, but algorithm. Could be a simple extra operation here or there, an extra variable that appears redundant. @mnewnham already made a gesture regarding the matter. Construct, not destruct. It is easy to diverge, to contradict. Simply arroganate, bring it from the self. But it is difficult to harmonize, to factor. The legacy of this world, all disposed from order to chaos. Hence, that is the easy. It takes years to build an edifice, and very short to take it down. It takes time to examine a current code, build a mental set of the features it utilizes, and restrict yourself to that, and no time to jolt straight in with code with whatever feature you want.

A commit, that brings loss and no gain. Question, can the new constructors be added without destruction? The answer has to be yes, but the assumption is that one already knew that, or everything said in the prior is meaningless. Could I be wrong, yes.

[valioz]

We are mixing two different tasks (1) eliminating PHP4 and (2) ensuring PHP7 compatibilty.
This pool request is small subtask of (2) and it is trying to solve only this change: https://wiki.php.net/rfc/remove_php4_constructors
If there are plans to solve task (1) in branch adodb6, then @mnewnham is right. PHP7 compatibilty must wait for it.
@CreatrixTeam is also right, this commit brings loss to task (1), but it solves part of task (2). Which is better for the project will decide maintainers, not me.
There is no need for more discussions. If this pull request is not acceptable for any reason, close it.

[EyeAndTea]

Thank you @valioz. We want, ourselves atleast, this pull request kept open. If you have not the time to rewrite the code per our advice we can try to give some of our time. By no means do we want this closed. We are very thankful to all contributions. One need not worry too much about us, we are not the maintainers. You are correct a number of issues are at hand. However we hope the volunteered effort is not fragmented unnecessarily.

[dregad]

@IPSO

Are recent versions of ADODB even actively tested on PHP4?

No. The current version of ADOdb is meant to support PHP 5.x only.

Any PHP 4.x specific code left in there is legacy code that should be cleaned up eventually, but hasn't been yet for lack of time/resources. As long as this code does not cause errors or warnings, it does not do any harm so this is a low priority task.

For the record, the original ADOdb4 code is available exactly as I have received it from John Lim, in a dedicated, unmaintained branch in this repository named adodb4.

@valioz's pull request is perfectly valid and something we definitely need to implement, whether in an upcoming 5.x release or in the next version remains to be decided.

In my opinion, since we already do not effectively support PHP 4 today (even though the code may work on it), I have no issue with merging this in 5.x as it is (althought I did not look into details or test anything at this stage).

I am also not opposed to amending this PR along the lines of what @CreatrixTeam suggested to maintain PHP 4 compatibility (i.e. use a __construct() function that calls the PHP4 constructor), assuming that it is necessary and provided that they (or someone else) does the legwork. And of course, without any implication that PHP4 is a supported platform.

[valioz]

If I understand correctly @CreatrixTeam suggested something like this:

    // function filter is not used as constructor in PHP 5+
    class Filter {
        function __construct($a) { $this->filter($a); }

        // PHP 5.0.0 - 5.2.13, 5.3.0 - 5.3.2: E_STRICT "Redefining already defined constructor"
        // PHP 5.2.14 - 5.2.17, 5.3.3 - 5.6: No error is raised
        // PHP 7: No error is raised
        // PHP 8: No error is raised
        function filter($a) { /* constructor code */ }
    }

According PHPRFC this will emit E_STRICT on PHP versions 5.0.0 - 5.2.13 and 5.3.0 - 5.3.2. I do not have such versions and can not test it.

@IPSO

Are recent versions of ADODB even actively tested on PHP4?

I made a test to start ADOdb 5.19 with PHP 4.4.2 and as I expected result is:

    Parse error: parse error, unexpected T_CLASS in adodb\adodb.inc.php on line 361

[dregad]

// PHP 5.0.0 - 5.2.13, 5.3.0 - 5.3.2: E_STRICT "Redefining already defined constructor"
// PHP 5.2.14 - 5.2.17, 5.3.3 - 5.6: No error is raised

So in other words, we'd have to choose between getting an E-STRICT for old 5.2.x and 5.3.x PHP versions or an E_DEPRECATED in PHP 7...

I made a test to start ADOdb 5.19 with PHP 4.4.2 and as I expected result is:

    Parse error: parse error, unexpected T_CLASS in adodb\adodb.inc.php on line 361

I guess this basically makes the whole point of even trying to maintain PHP4 compatibility moot...

[valioz]

So in other words, we'd have to choose between getting an E-STRICT for old 5.2.x and 5.3.x PHP versions or an E_DEPRECATED in PHP 7...

Тhat is why I prefer to not accept this request if we want to keep PHP4 constructors in this version, although I do not see the benefits of this. Maybe it is better to have a new branch adodb6, as proposed by @mnewnham.

[Mike-Benoit]

As @dregad said, any discussion regarding PHP4 compatibility is a waste of time as ADODB v5.19 already doesn't come close to supporting it... So this PR basically cleans up existing code and fixes some PHPv7 warnings at the same time, it would be a welcome change as far as I'm concerned.

Whats the benefit in creating a ADODB6 branch if the current branch already doesn't support PHPv4 and this PR won't break any existing PHPv5 installs?

[dregad]

Whats the benefit in creating a ADODB6 branch if the current branch already doesn't support PHPv4 and this PR won't break any existing PHPv5 installs?

That was my point...

[EyeAndTea]

Thank you, all participators.

After @valioz revelation above of the syntax error and PHP versions 5.0.0 - 5.2.13 and 5.3.0 - 5.3.2, we can no longer give a decisive formal defense for our position.

However, much of what we said earlier still stands on logical grounds. Mainly, avoid divergence, seek harmony, factor, pattern, converge. The code at its current state is likely far closer to PHP 4 than PHP 7. In one end of the spectrum, that syntax error could be the only error. In the other end of the spectrum, the code is actually closer to PHP 7 than PHP 4.

We still advice against such a commit. As a PHP 5 code, the code works on PHP 5 without the commit. As grounds for a progressive commit, the code is not grounds for such a commit. In other words, the current code suffers from much deeper issues all the way to the architecture level. The code is fragmented. It is non homogeneous. No clear specification exists, and apparent specification can be extremely difficult to deduce. Let us fix these issues before we introduce further divergence. It should be noted that this is currently where all our current effort is being spent, and hope the focus is targeted to such matters first.

[Mike-Benoit]

@CreatrixTeam with all do respect your argument doesn't hold any water.

You don't want to change the code because it may diverge from PHP4 code (which doesn't work anyways) at the cost of not conforming to "proper" PHP5 constructor code as well at the cost of preventing PHP7 compatible code. Your entire argument in this thread has been based on "hand waving" and guesses. @valioz is the only one who did the work to determine if ADODB v5.19 even works on PHP4, and it doesn't.

It makes no sense to spend any more cycles trying to "get close" to supporting a PHP version that has long been unsupported at the cost of preventing the code base from working on a future version of PHP. There is a ADODB4 branch if you want to continue PHPv4 support, outside of that branch discussing PHP4 at all seems like a total waste of time.

I'm all for any reasonable attempt to get the code to work on as many versions of PHP as possible, and this change brings the code inline with existing PHPv5 standards and it just so happens to allow the code to be more compatible with PHPv7, which is a win-win situation. I can't see how this could be argued at any other way.

[dregad]

Fixed in 9ffa02e

[EyeAndTea]

We do wish the commit could have waited for the time being. This has unnecessarily put us in quite a difficult position.

[peterdd]

@CreatrixTeam Why, if adodb doesn't work with php4?

[dregad]

@CreatrixTeam I think we have clearly established in this thread that PHP 4 is not supported in this release of ADOdb. Your original point is therefore moot, as I mentioned previously. As for your difficult position, maybe you should have started by clearly stating what the problem was instead of waving your non-constructive theories about harmony, reduction and backwards compatibility..

You mentioned wanting this PR to remain open, while offering your time to rewrite the code in a different way; nearly a month has passed since then, but you did not give any further feedback or pull request to that effect.

Since @valioz's submission fixes a valid problem and you were not able to provide any sound argument against it, I saw no reason to delay the implementation. If you are not happy with the way we maintain this software, you are welcome to try and do it better in your own fork.

[EyeAndTea]

Good to hear from you, project maintainer.

We indeed did not provide a code alternative for this submission, because of @valioz sharp revelations and in particular those about PHP versions 5.0.0 - 5.2.13 and 5.3.0 - 5.3.2.

We spent much resources working on the architecture of the library. Our code base was becoming much more homogeneous and well defined. Many drivers benefited without adding new code to them related to the benefit, for example. A homogeneous fully factored code is far easier to harvest effort on, and far quicker to improve. And in all our efforts, and listen carefully, we did not go after one driver or another, one PHP version or another, but simply followed what constituted the current library. Some drivers were PHP 4 drivers. Some code properties, and the our following code guidelines, were PHP 4 pertaining. Yes, perhaps these PHP 4 drivers were not on your mind, but we spent time and effort on them. Say they are idiots if you like, but that is where their effort went and you can not factor code without examining the entire base.

This commit diverged the code considerably, and is a commit that could have been done at any point. Also the library without the submission, works as a PHP 5 library. Is one wrong. We were not planning to stay forever, you could have waited till we submitted the fruit of our efforts, and after that you could have done what you want. The project is yours. We did not add single mention of ourselves anywhere in the code we added. But we were addressing things, and fixing things that should have been fixed a decade ago, and yet this is what could not have waited a bit more.

A choice that frustrates professional efforts over something whose absence was not an immediate loss, and implementation a simple manual labor, and presence a gain yet to be attained, is unwise. Perhaps one is missing something.

We still thank to @valioz for his submission, if he is reading this.

This is our last comment on this matter.