![Popular open source software is more secure than unpopular open source software, because insecure software becomes unpopular fast. [That doesn't happen for proprietary software.]](../sites/all/themes/drupal4hu/images/bg-center/bg-center_4.png)
Right now we have something like hook_user_logout which is documented in user.api.php and if mymodule wants to implement it then it needs to use a function called mymodule_user_logout. This is a Drupal convention and is not supported by any developer tool. We could instead have an interface called HookUserLogout with a single method called invoke which has the exact same signature and documentation as hook_user_logout. Edit: aside from IDE support, this would mean that PHP itself forces us to keep the hook signature. Invoking would happen through the registry table where we iterate over the classes implementing this interface. Of course, this would be cached but module_implements uses a cache in Drupal 7 already so not much change there. Code example is here, a skeleton of the invoker is here. The conversion would involve wrapping the function in a class, that's literally two lines of code including the } and renaming the function to invoke. The cost of creating a class vs calling a variable named function is not negligible but simply there are not enough on the page to make a measurable difference: 1000 calls are like 0.0025 vs 0.003 seconds.
Commenting on this Story is closed.
Yes that does look enticing from the outset. However you are talking about an 8 times increase in execution, and the amount of code that will need to be changed would be huge, not counting code in contrib modules just to give a much better method of self documentation within IDE's
I think the cost of doing this is out weighed by the benefit that it will provide.
I do like this, and would be nice, but it really needs more benefit. Could we add more than just 1 method to a class to allow flexibility that is currently not available.
Maybe something like a access method that will get executed to check it this should be executed.
Maybe this could be discussed more at SF.
Gordon.
More like 20%.
Detecting interfaces from static analysis is substantially harder than you suggest. Remember that a class may implement an interface indirectly by implementing another interface that extends it, or by extending a class or abstract class that implements that method (which in turn could implement it in either way, etc.) PHP's reflection library can certainly do all of that hard work for us, but only if we first parse the file into memory and execute it. We ran into that issue with the registry originally, which is why it uses static analysis.
And at that, I don't think the code gets appreciably better. There's no improvements in loose coupling, one of the key reasons to make something an object or interface. All we really gain here is potentially auto-completion on a single method... which actually we don't get since that doesn't work on definition, only on invocation. And since we're still using module_invoke_all(), we don't get it on invocation.
The advantages I would see here are 1) We can autoload "hook" implementations with no extra work. 2) A single module could have multiple implementations of a hook if it needs to do 2 different things, which is a modest DX improvement. (There are plenty of places where we could do more to improve DX with OO than that, though, like anywhere we have magic callbacks.)
This really sounds like a clever language hack, not an architectural improvement. Drupal is well past the time in its history where "clever language hacks" can substitute for good architectural design.
<?php
$moduleinstance->hook = function() {
}
?>
no introspection, no clever naming tricks.
What is $moduleinstance? Modules are still not classes, you know. Anonymous functions would require 5.3 (that's what you've used, not a closure).
Anonymous functions won't help here for many reasons.
First off, you can't bind an anonymous function to an object at all in PHP 5.3. That was deliberately left out because the internals team hadn't decided yet how they wanted it to work, so they just said "not happening" for 5.3 to revisit in PHP.next. The consensus since then is very clearly that people do want to be able to bind closures to an object so that they become a pseudo-method, which is awesome, but won't happen until PHP 5.4 (or whatever it gets called).
Secondly, that doesn't help at all with registration. We need a way to register what hooks are available for to be called that is fast, simple, and cheap. In Drupal 6, we have function_exists() and, that's it. In Drupal 7 we have function_exists() with an assist from hook_hook_info(), which helps a bit but we're not really using it yet.
Using "does this $module object have $module->hookname defined on it" is slower than just function_exists() because you first have to create all of those anonymous functions, and doesn't give you any more robust registration options. You still need to parse and then execute *everything* before you can detect what hook implementations are available.
I've just think about chx's idea, and made few modifications (it's solves the function registration problem and makes it more robust):
- the module part of the "new hook system"
- the Drupal part
Your numbers are off; I did some pretty extensive microbenchmarking recently (http://blog.samboyer.org/blog/microbenchmarking-php-performant-code-now-20-less-superstitious-handwaving). Class instanciation plus method call is likely going to be more in the realm of three to five times as heavy as a single variable function call. And I suspect the number of times it's called could get quite a bit above 1000 for a site of any complexity, in which case we could be talking about an extra 20, 30ms purely for this system. Not a worthwhile tradeoff, given Crell's points (which I agree with).
I like your thinking about updating the way we do hooks. I'm not sure what you suggested is the best way but we should keep looking at our options.
I can’t agree with you more!