Noop function called and WP_Dependencies constructors conceptually broken

[hakre]

See [15639]

In PHP4, extending a class and providing the PHP 4 constructor function (function named as class) will override the existing, complex constructor structure that is part of class WP_Dependencies.

So at first, the constructor is conceptually broken:

In #10861 it was argumented the complex structure is for extending, this little PHP 4 code demonstrates that this principle does not work:

<?php
/**
 * PHP 4 constructors and inheritance
 */
class foo_parent {
	function foo_parent() {
		printf("%s()\n", __FUNCTION__);
		$this->__constructor();
	}
	function __constructor() {
		printf("%s()\n", __FUNCTION__);
	}
}

class foo extends foo_parent {
	function foo() {
		printf("%s()\n", __FUNCTION__);
	}
}

$test = new foo();
?>

Output:

X-Powered-By: PHP/4.4.2
Content-type: text/html

foo()

As the constructor only need to call itself (if it's non-overridden), it is already called. If the extending class needs it's own constructor, it only needs to define it. In this case, the parent's constructor won't be called any longer.

Next to the fact that it does not what it is inteded for, PHP 5 is compatible with PHP 4 constructors, so there is no need to introduce a expensive variable function call to a function that is NOOP ing.

But that's not the end. As child classes (e.g. WP_Scripts) use the PHP 5 constructor only, their parent implementation is not called making it useless. Instead they only need to implement their PHP 4 constructor. to have the same behavior (variables can not be passed by reference with variable function calls via call_user_func_array()).

The current implementation is hard to understand, conceptually broken and makes the code hard to read.

[filosofo]

Replying to hakre:

In #10861 it was argumented the complex structure is for extending, this little PHP 4 code demonstrates that this principle does not work:

The code can't demonstrate that, because the point of the hack is for descendant classes not to use their eponymous constructors; instead, the point is for them to use the PHP 5-only __construct constructor.

But that's not the end. As child classes (e.g. WP_Scripts) use the PHP 5 constructor only, their parent implementation is not called making it useless.

Actually, in PHP 4 if a descendant class has no eponymous constructor, at instantiation the parent class's eponymous constructor is called. So it is not useless as it does what is intended.

The current implementation . . . makes the code hard to read.

Acknowledged. But it's a hack to make PHP 4 syntax slightly more like PHP 5's, so there's no reason to remove it prior to making PHP 5.x the minimum required version, which will come soon enough.

[hakre]

Replying to filosofo:

Replying to hakre:
[...]

I read your comment that you assume that it's wanted to provide __construct() as a constructor. But this is not needed. No-one needs this "hack", it's just added code that can be saved.

Sure it can be refactored after a switch to PHP 5 (saving it as well) at least then (with the switch to PHP 5) it's obvious for everybody.

Until then it remains in there throwing sand into the readers eyes as if this had some special meaning. It's just a bad written constructor construct that can be better implemented by using the existing language feature w/o re-inventing the wheel. The patch just showing a way how.

[hakre]

Related: #14921

[nacin]

WP_Dependencies can optionally be extended elsewhere, in addition to just WP_Scripts and WP_Styles.

Closing as wontfix. When we remove cruft in 3.2, I guarantee every constructor will get a good look and we can handle it then.

[hakre]

Replying to nacin:

...
Closing as wontfix. When we remove cruft in 3.2, I guarantee every constructor will get a good look and we can handle it then.

I can live with that. Right now the __constructor() is already overridden by PHP 5 as it is defined in extending classes.

I suggest to reopen and put this on the 3.2 milestone. Or better to re-open #10861 then.

Tag: MIGPHP5

[hakre]

Related: #16768 (current ticket)