Hey,
A lot of components in core, for instance SpecialPages, API modules and Actions allow registering new components as follows:
$someList['some-name'] = 'YourHandlingClass';
The problem with this is that the extending code has no control over the instantiation of the handling object. So you can't inject any dependencies. That is more then a little bad design wise.
== Dependencies need to be pulled ==
Even when trying to properly design your handler, you are forced to have some static calls to obtain your dependencies (at least one if you have any dependencies) for no good reason. I don't see any way to get from under that. Furthermore this design simply encourages pulling in dependencies, so this is often done, regularly involving global state (not talking about global variables here, global state in general), thus seriously hurting the quality of the wider codebase.
== Cannot register a handler with state ==
You are forced to create different classes while different instances of a single class might suffice. Of course that just works in some cases, often you have to give up on the proper design altogether to make it work with the static registration system. This limitation seems to encourage abuse of inheritance as well, though it's certainly not the only thing doing that, and is not the focus of this mail.
Are there any ways for the mentioned components (SpecialPage, API and Action) to do have control of the lifecycle of the handling object that I am not aware of? Does anyone have plans to address these issues in our internal API, or has any work been done in this direction already?
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. --
On Mon, Feb 18, 2013 at 8:41 AM, Jeroen De Dauw jeroendedauw@gmail.comwrote:
A lot of components in core, for instance SpecialPages, API modules and Actions allow registering new components as follows:
$someList['some-name'] = 'YourHandlingClass';
The problem with this is that the extending code has no control over the instantiation of the handling object. So you can't inject any dependencies. That is more then a little bad design wise.
This all ties in with the class autoloader and the decision years ago to do lazy-loading for code rather than preloading all classes and functions. By specifying a class name rather than a live object, we let the actual code for the class sit unloaded until something actually uses it.
So if that's something we want to maintain -- and it helps performance *a lot* for small installs when there's not an opcode cache in use -- it's worth thinking about ways to inject your data without having to instantiate much live code.
One way would to specify a callback. For PHP 5.3 and later we can use closures...
$blah = $something; $someList['some-name'] = function() use ($blah) { return new YourHandlingClass($blah); };
and then have the instantiating code check if it's been given a callable function rather than a class name. Since this doesn't run the function until it's used, the autoloader still doesn't load the class until it's actually run.
-- brion
Hey,
One way would to specify a callback. For PHP 5.3 and later we can use
closures...
That would work yes! :) I've been doing something very similar for various hooks lately, and it works great.
$this->hooks['ParserFirstCallInit'][] = function( Parser &$parser ) use (
$extension ) {
$hookRegistrant = $extension->getHookRegistrant( $parser );
$countHandler = $extension->getCountFunctionHandler();
$hookRegistrant->registerFunction( $countHandler );
return true;
};
Where $extension is a very basic DIC.
Anyone objections, concerns or further thoughts on Brion's suggestion?
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. --
Hey,
I had a go at this and have implemented a working solution for SpecialPages, Actions and API modules. Slightly surprised at how easy this was considering some of the involved code is procedural soup with a strong taste of global state.
* https://gerrit.wikimedia.org/r/#/c/49777/ * https://gerrit.wikimedia.org/r/#/c/49781/ * https://gerrit.wikimedia.org/r/#/c/49785/
So assuming the arguing about typos and bracket placement takes average time, it ought to be in somewhere round MW 1.25 :)
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. --
wikitech-l@lists.wikimedia.org