The future of hooks

[bfncs]

Hello fellow wirecionados,

looking at the roadmap to 3.0 always gives me the good feeling of being on the sunny, futureproof side when using Processwire. Everything we already have together with the features planned is all I ever needed from a CMS. At least this is what I thought until that next idea came to my mind.

Actually, there's one thing about Processwire that I have been struggling with from the start and until now: the hook system.

I know, the hook system is deeply enrooted in Processwire (as can be seen from the fact that it's implemented in the Wire class that virtually every other class extends) and therefore is one of the key aspects for its success in the first place. It allows for the relatively loose coupling of components, with a lot of the core functionalities being implemented as modules. This makes for the great extensibility that Processwire is famous for.

There aren't many parts of Processwire that can't be tweaked and modified to anyones needs by developing a module that alters some core functionality. I'm really glad about that and probably wouldn't have started to use Processwire in the first place, if the hook system didn't exist.

That all being said, I also see some more or less problematic downside to the current implementation.

1. It's sometimes pretty hard to find out, where functions are actually implemented. How long did it take you to find out that the actual logic for Page::render() is implenented in PageRender::renderPage()? While this kind of flexibility is just great, the implementation is not very verbose. This makes things harder than they have to be – especially for beginners.
2. Just to make the argument above worse, there are several different ways to define a hook: addHook(), addHookAfter() & addHookBefore(), and all can be called either with a Class name or on an object. This makes it even harder to find modules, that hook into a particular function. I know, we have Soma's fabulous Captain Hook, and I use it all the time, but it still feels more like a workaround than like a solution.
3. Speaking of Captain Hook, there's still a „Crocodile Hooked“ missing, a central repository that keeps track of all methods that are hooked to another method, including (private) site modules – and if possibly available at run time. Hooks for classes are saved in a static variable of Wire, but local hooks can only be fetched from that particular instance of an object.
4. The convention with the three underscores in front of a function name makes functions harder to find.
5. It is a nightmare to debug. If you ever tried to follow a request through the system with Xdebug, maybe involving some hooks placed by modules, you probably know what I mean. You have to go through Wire::runHooks() and __call() again and again to get where you want.
6. It also prevents some of the convenient features of your favourite IDE™ (e.g. code completion, parameter hints when using a function, go to the definition of a function by clicking on it, etc.) from working in some cases. While one could definitely argue, that this is the fault of the IDE used, it's still slowing down work and not likely to be changed anywhere soon.

Please excuse me if this was too ranty and even more correct me if anything I mentioned is an error on my side. I have to emphasize that none of these downsides prevents anyone from getting something done, it just makes it a tad less obvious and comfortable.

Thinking through the things that I don't like about the current implementation of the hooks system, it came to my mind, that what we currently have here is kind of a non-verbose version of a „traditional“ event system and maybe what's missing could be more verbosity, so maybe such an event system (that implements the mediator pattern) could be of help?

What I have in my mind here is the Symfony EventDispatcher or the Drupal8 EventDispatcher component, which is similiar to the CakePHP Events System and the handling of Events in Laravel.

Let's look at how a solution to the problems mentioned might work by way of example using the Symfony EventDispatcher.

1. In this particular example, the difference would be that Page::render() would actually be implemented in the Page class but just dispatches an Event.render event. Functionally no real difference, but much more verbose and therefore it is much more clear, that you will find the implementation anywhere a listener for Event.render is added.
2. A new implementation could use just one method to add listeners (e.g. EventDispatcher::addListener()). Execution before or after an event will have to be handled by multiple events where this makes sense.
3. Because the event dispatcher is the application-wide sole instance where event listeners are stored, it's easy to get all registered event listeners at runtime (using EventDispatcher::getListeners()).
4. No more underscore's, just calls to dispatch events and calls to listeners.
5. Debugging get's as smooth as it get's with loose component coupling. No more running through __call() all the time, always be able to check the defined event listeners. Having separated before and after events prevents you from some running through runHooks(). The added verbosity also helps to grasp the existing architecture quickly.
6. Most of the IDE comfort functionality that doesn't work now should work with the Event System in place, because functions are always really where they are expected and with the exact same name.

As an added benefit this system is very well known to a lot of developers already. But there are also some downsides for sure:

• Added verbosity is, well, verbose. More keys will have to be hit to get something done.
• Method and property would have to be implemented differently. This can be done by sending events on missing methods/properties, but it certainly looses some of the appeal it has now.
• The key argument would for sure be, that it breaks compatibility with a lot of core and user contributed modules. This is definitely true, so there shouldn't be an easy decision on this one.

I thought I'll use the time before 3.0 to bring this one up, because as I understand it, the next major will break some stuff either way. Also I think that it should be possible to do the switch by in the beginning only deprecating the old hook system while keeping it functional, by translating calls to the old functions to adding and calling event listeners. This will have a performance impact, but would make a gentle transition possible.

Now I'm really interested on your opinions on this topic. Do you already like the current implementation and wouldn't want to loose it? Do you have other problems with hooks? What kind of solution could you imagine? Is such a deep change realistic (and worth it) at all?

Thanks a lot for your feedback

Marc

[LostKobrakai]

It's an interesting topic you've mentioned here. I'll just want to take some of your issues with the current system into my perspective:
 

How long did it take you to find out that the actual logic for Page::render() is implenented in PageRender::renderPage()

 
Right on top of the Page Class you'll find this. It's not the most verbose, but a simple search for render() will get you there. Also I'd think that there's a reason, why the function isn't in the Page Class.

/* Methods added by PageRender.module: 
 * -----------------------------------
 * @method string render() Returns rendered page markup. echo $page->render();

Just to make the argument above worse, there are several different ways to define a hook: addHook(), addHookAfter() & addHookBefore()

I can see, why the first function could be difficult to beginners, as it could cause problems, if one would need the hook to run after or before. But I just never used addHook() in the beginning, because I wanted to be sure it's running before or after the hooked function, because there where lots of other potentials of error, so I tried to at least force the hook to run on the expected time. 

The convention with the three underscores in front of a function name makes functions harder to find.

 
Can you elaborate on that? I really can't see, how those underscores would alter the ability to find a function.
 

In this particular example, the difference would be that Page::render() would actually be implemented in the Page class but just dispatches an Event.render event. Functionally no real difference, but much more verbose and therefore it is much more clear, that you will find the implementation anywhere a listener for Event.render is added.

That could also be the case with the current hooking system. Page::render() could be a function of Page, but it's "added" to the object. I don't know the event system you're mentioning, but if it needs a base function to dispatch an event, how would one add new functions, that aren't there ahead of time? Additionally by now one just adds the underscores and doesn't have to care more about anything and it's hookable. If I understand you right, this wouldn't be the case with those events.

[owzim]

Interesting topic, thanks for your efforts accumulating all those infos.
 
Although I've yet to investigate further the alternatives you are proposing, there are a couple of things that come to my mind regarding this:
 
When it comes to debugging hooked methods, I totally feel you. It can make you pull your hair sometimes.
 
But the thing is, you can either have massive flexibility and power in this case, and then have to trade in some other things. It's like with loosely types languages vs. strongly typed languages.
 
Loosely typed languages give you great power, but it can be a pain, for let's say a Java developer.
 
Two things I've used heavily in the past half year or so, that saved me a lot from going bald:

• The ChromePhpLogger module by soma, it's not an ideal debugging tool on it's own, but certainly a necessary one in my toolbox
• Unit tests: I've learned to split my logic into pieces as tiny as possible. During development I only test against those tiny pieces. If the pieces are working on their own, the big picture code will mostly work out of the box. Debugging also only happens on the tiny piece level and makes massive backtrace debugging madness almost vanish

[Wanze]

Interesting discussion!

For me (as a developer), the advantages of ProcessWire's hooking system overweight the issues you mentioned. I agree, it's sometimes harder to find the relevant piece of code, because it gets added dynamically with hooks. Debugging is definitely harder too.

But in my opinion you can never achieve the same flexibility with events. For example, how would you handle the beforeHooks, which are executed before the "real" method is invoked? How would you modify the passed arguments? Also the afterHook can modify the return value. It's so powerful and easy to do - just add those three little underscores and you can use this power in your own Wire-derived classes. I like

[BitPoet]

After having implemented a few different event dispatching systems in my programming carreer, I've still not found the perfect approach, but I've very much given up on overly declarative ways (which do include centralized event management) because at some point, they tend to introduce a level of coding ugliness that's without second. Where, in PW, I can add a hook the moment before it is needed if circumstances are right, a declarative approach often needs loads and loads of repetitive code inside the event hook to make sure its logic is only executed when necessary.

Also, declarative event syntax tends to necessitate carrying around much more context, which can be detrimental to memory consumption and performance. Mediator patterns are good in theory, but everyone who has worked on huge event driven projects in C# or Java knows that it's not just a few lines of code more - if you're consequent, you're going to be writing a few times as much code and still find yourself making assumptions about needed interfaces that aren't matched by reality.

To me, the biggest issues I have with PW's hook system are ones that can be solved without taking a wholly new approach:

• Hookability: there are still too many unhookable methods, but that's often because of the next point on my list, which is:
• Granularity: like with all growing codebases, often code gets written that "gets the job done", yet it does more than one thing at once. Most headaches I get arise from the fact that the hook I need would have to be called somewhere in the middle of an existing method. Also, for debugging reasons, wrapping individual changes inside their own, aptly named hookable method would go a long way;
• Statefulness: not every state transition has a hook, and if it is there, some information about what the state change(s) entailed may be missing at the next hookable point, or not convenient to get at, and even more, it's often difficult to discern how to revert the state transition. These are usually small things in the scale of PW (and happen far less than in other frameworks I've worked with) as a whole but will pop up every so often as the codebase evolves and grows. No event system will catch that on its own either;
• I was about to write "hook execution order" before I felt a murky memory tickle the back of my mind and decided to take a look to make sure - lo and behold, hook priority has already been there for years. Yay, Ryan!

This is no criticism on PW btw., just a reflection of what points keep popping up. Like I wrote, these are the same things that come up in every big and growing software project.

I'm very much with owzim that the only way around debugging madness is granularity. No event system will cure you of memory exhaustion or eye soreness when the backtraces keep growing and the objects to inspect are deeply nested.

Once runkit_method_add or a similar mechanism becomes stable, it might make sense for debugging purposes to (optionally) augment the Wire base class (or an event manager stand-in singleton) with static hook closure methods following a fixed naming theme that a debugger can look at, which would alleviate one of the main issues in the OP.

[apeisa]

I agree with with BitPoet. There are few methods and classes that would benefit hugely with rewrite where there would be more and simpler methods. For example ProcessPageEdit is pain to hook into...

Overall, hooking system is beauty!

[bfncs]

Wow, thank you all for the very deep insights you shared! You really helped me made up my mind on the topic. I'll try to answer some of the direct questions and main arguments discussed.

Can you elaborate on that? I really can't see, how those underscores would alter the ability to find a function.

Sure, I'll illustrate it with an example: if you search for the function Pages::saveField()and use a simple grep search like grep -R 'function saveField' wire/ you won't find it. A simple grep -R 'function _*saveField' wire/ gets you there, anyway, so it's not that big of a deal.

I don't know the event system you're mentioning, but if it needs a base function to dispatch an event, how would one add new functions, that aren't there ahead of time? Additionally by now one just adds the underscores and doesn't have to care more about anything and it's hookable. If I understand you right, this wouldn't be the case with those events.

Adding new methods with an event system wouldn't be much different from the current implementation in terms of functionality. Pretty much like in the current implementation it could be possible to dispatch an event in __call() when a method called is not defined. A listener would then be executed and cary out the implementation.

But in my opinion you can never achieve the same flexibility with events. For example, how would you handle the beforeHooks, which are executed before the "real" method is invoked? How would you modify the passed arguments? Also the afterHook can modify the return value.

An event-system-driven implementation wouldn't remove any of the actual flexibility. Before hooks could be implemented by dispatching a respective event in the first line of a function. Passed arguments would be added to the Event object that is passed passed to the EventDispatcher and could be modified by all listeners. The same would be true of after-events with their respective return values.

Reading through all these arguments it became more and more obvious to me, that what I was proposing isn't that different at all from the current implementation. Essentially there are just two differences: added verbosity and a central event authority (which we also nearly have already). This would have some benefits and some drawbacks, so it's really questionable whether it would be worth the hassle of converting.

Also, declarative event syntax tends to necessitate carrying around much more context, which can be detrimental to memory consumption and performance. Mediator patterns are good in theory, but everyone who has worked on huge event driven projects in C# or Java knows that it's not just a few lines of code more - if you're consequent, you're going to be writing a few times as much code and still find yourself making assumptions about needed interfaces that aren't matched by reality.

Well this is definitely a big drawback that I didn't think of until now. I don't have as much experience as you have with rolling out an event system in a big application and mostly know it from the "consumer" side of things, but your argument seems valid and really important here.

• Hookability: there are still too many unhookable methods, but that's often because of the next point on my list, which is:
• Granularity: like with all growing codebases, often code gets written that "gets the job done", yet it does more than one thing at once. Most headaches I get arise from the fact that the hook I need would have to be called somewhere in the middle of an existing method. Also, for debugging reasons, wrapping individual changes inside their own, aptly named hookable method would go a long way;
• Statefulness: not every state transition has a hook, and if it is there, some information about what the state change(s) entailed may be missing at the next hookable point, or not convenient to get at, and even more, it's often difficult to discern how to revert the state transition. These are usually small things in the scale of PW (and happen far less than in other frameworks I've worked with) as a whole but will pop up every so often as the codebase evolves and grows. No event system will catch that on its own either;

 

What you mention here pretty much hits the nail on the head. Solving the issues you brought up, especially the last two, would probably completely solve the need for a change that I felt in the first place – and probably even more than an event system could ever improve the overall architecture.

This also looks to me like a pretty big project that would introduce breaking changes as well. In theory it should be possible to build a smooth transition by keeping non-granular functions and only deprecate them. The question is how we could approach this and how we as a community could help get that transition done. Any ideas?

• I was about to write "hook execution order" before I felt a murky memory tickle the back of my mind and decided to take a look to make sure - lo and behold, hook priority has already been there for years. Yay, Ryan!

Haha, I went through the exact same process when writing the initial post. Cheers, Ryan, for always being a few steps ahead!

This is no criticism on PW btw., just a reflection of what points keep popping up. Like I wrote, these are the same things that come up in every big and growing software project.

I hope I made that clear in the first place, but just want to emphasize it again: Processwire is by all means the greatest CMS I know and I can't tell you how much I appreciate the work by all contributors, especially Ryan.

With the next major version on the horizon, I just just think it's a great opportunity for the whole community to think about the small tads that can still make it an even better overall experience.

Once runkit_method_add or a similar mechanism becomes stable, it might make sense for debugging purposes to (optionally) augment the Wire base class (or an event manager stand-in singleton) with static hook closure methods following a fixed naming theme that a debugger can look at, which would alleviate one of the main issues in the OP.

Good that you mention runkit_method_add, because I know that I read about it some time ago and then completely forgot about it. Yes, this would enable much nicer ways of debugging than any event system could provide, so it's probably the best thing to improve the code (granularity) with the system we already have and wait for the runkit extension to become stable to improve it further.

Thanks you all for the great discussion.

[Wanze]

An event-system-driven implementation wouldn't remove any of the actual flexibility. Before hooks could be implemented by dispatching a respective event in the first line of a function. Passed arguments would be added to the Event object that is passed passed to the EventDispatcher and could be modified by all listeners. The same would be true of after-events with their respective return values.

Ok you're right. But I guess this would work only nice for parameters that are passed by reference, which in PHP are objects. Parameters like strings/ints are passed by value (unless using the prefix "&"). If an event listener does change the value, this does not affect the variable in the actual method. Maybe I still don't understand what you mean  

[bfncs]

Wanze, I think an event implementation would have to handle this in one of the following ways:

Either by attaching arguments by value to the Event object and later reassigning changed values to the variables (definitely too verbose). Or by passing parameters by reference, though this might be a bit unconvenient to do (one function call per assignment, at least until we can safely use the new variadic functions splat operator of PHP 5.6 with pass by reference). The third, most convenient way would involve the usage of __call(), much like it is already done in the current implementation – and therefore defeats most of the benefits gained by an event system in the first place.

I'd say that this is one of the drawbacks of a verbose implementation that an event system approach would force on us, and a serious one. Thanks for pointing it out, it helps me appreciate the current implementation more and concentrate on the real weaknesses pointed out by BitPoet above.

[ryan]

I know there are benefits and drawbacks to different approaches, so I'll try to explain why PW uses the approach it does, and respond to some of the points above.

The approach described as an alternative here is similar to what I've used on other projects, including PW1. I was really looking to come up with something different for PW2. The goal was to make it as absolutely simple as possible for people to add hooks and for people to make methods hookable. I wanted it to be in the background and everywhere, so that you really didn't even have to think about it.

It's not designed for ease of consumption by xdebug, I'll give you that. But it is designed for ease of consumption by you and me, which I consider more important. Admittedly I don't use xdebug very much anymore, so designing things for xdebug has not been a priority. When you start a new CMS project, you are in a crowded space, so you have to come up with things that are different. So one of the ways ProcessWire differentiates itself is by making hooks easier to hook into and create than they are in other platforms, and I'd like to keep to that.

When it comes to PW's code base, I consider it important to not add complexity purely for sake of static analysis. From my perspective, static analysis is a good thing, and I'm more than happy to let it dictate the direction of comments and what supports the code. But when it starts to dictate the direction of the actual code towards unnecessary complexity/verbosity, less efficiency or reduce the simplicity of the API, then I consider it no longer helpful. If ProcessWire were just a PHP framework and coders in IDEs were the primary audience, then I would feel differently. But for ProcessWire's audience and growth, when I'm confronted either making my IDE happy (and related tools like xdebug), or making the API (or related things like hook system) as simple and as good as they can be, I'm always going to choose what's best for PW's API and systems. My preference is making both happy. But when you are doing something different, existing tools won't always be familiar with it. For something like the hook system, there was definitely a conscious decision to go with what's best for PW and PW users, over what's best for static analysis or xdebug. 

there are several different ways to define a hook: addHook(), addHookAfter() & addHookBefore(), and all can be called either with a Class name or on an object. This makes it even harder to find modules, that hook into a particular function. I know, we have Soma's fabulous Captain Hook, and I use it all the time, but it still feels more like a workaround than like a solution.

All the add hook methods are actually handled by just one method: addHook(). The point of the other methods (mentioned below) is to make things simpler for you, and make your code more readable. I prefer verbose method names to boolean and array arguments because they are self describing. think this is a benefit, not a drawback. 

• Use addHook() if you want to add a method to a class that isn't already there. 
• Use addHookProperty() if you want to add a property (rather than a method) to a class. 
• Use addHookBefore() if you want to hook before an existing method (and have the option of modifying arguments sent to it).  
• Use addHookAfter() if you want to hook after an existing method (and have the option of modifying the return value). 

Regarding the distinction of hooking a class name or an object, this is also meant to be a help to you: 

• The purpose of being able to call a hook on a class name is so that you can hook ALL instances of that class (current and future). 
• The purpose of being able to call a hook on an object is to hook only that single instance of the object. 

As for ease of finding hooks, this is also something I've tried to make as simple as possible by using a convention that you won't see elsewhere in ProcessWire: 3 underscores prepended to a method. So while you can use CaptainHook, I'm not sure you need to, because you can find all hookable methods in ProcessWire simply by doing a search for 3 underscores "___". A recursive grep is one easy way to accomplish that. 

It's sometimes pretty hard to find out, where functions are actually implemented. How long did it take you to find out that the actual logic for Page::render() is implenented in PageRender::renderPage()? While this kind of flexibility is just great, the implementation is not very verbose. This makes things harder than they have to be – especially for beginners.

 

Page::render() is actually a very unique case that you won't see elsewhere in PW (that I can think of). Usually I would have just put a method like that directly in a class and have it handle the render from there (or have the method there to call up the PageRender module). But PW started out as fully API driven, where there was no such thing as "render" of anything, to the core. So the ability to "render" anything was seen as something specific to the web context of a CMS. That's ProcessWire now, but its roots are pure CMF. Today, I would likely just put that render() method directly in the Page class. But I also don't see any reason to move what's already working there, as it really makes no difference in how it works. And you can still hook Page::render if you want to, just like it actually did exist in the Page class. Because you can't actually see a Page::render() method in the class, we use phpdoc at the top of the file for it. 

Hookability: there are still too many unhookable methods, but that's often because of the next point on my list, which is:

I would love to make everything hookable, but the reality is that there is a little overhead associated with every hook (in any hook system). So I don't usually make methods hookable unless there is a request for it to be. We started with very few hooks, and now we have quite a lot. But the majority of the hookable methods you see in PW today are the result of people asking for them to be hookable. And luckily, it's very easy to do just that: just append 3 underscores, and you are done. 

Granularity: like with all growing codebases, often code gets written that "gets the job done", yet it does more than one thing at once. Most headaches I get arise from the fact that the hook I need would have to be called somewhere in the middle of an existing method. Also, for debugging reasons, wrapping individual changes inside their own, aptly named hookable method would go a long way;

I would answer this pretty much the same way as the Hookability point above. When more granularity is needed somewhere, we add it, and there's lots of examples throughout the core. But it's hard to know where one might need that until it is asked for somewhere specific. When a module like ProcessPageEdit was originally written, I honestly didn't know if anyone would ever want to hook anything in it at all. Over time, it has become more granular, and over more time will become even more so I'm sure, but only when specific needs for granularity/hooks come up. I love flexibility, but I also love efficiency. If everything in ProcessWire was fully granular and hookable PW might be very flexible for someone looking to hook specific things, but it would also be bloated and slow. My preference is to make specific things more granular and hookable when it will solve someone's specific need. Granted, there are plenty of hooks in PW already that nobody is using, but they are generally in places where it makes no difference in terms of efficiency. I'm not suggesting that hooks aren't efficient – there's very little overhead from hooks. It's just that if you multiply a very little overhead over thousands or hundreds of thousands of calls, then it becomes something.

Statefulness: not every state transition has a hook, and if it is there, some information about what the state change(s) entailed may be missing at the next hookable point, or not convenient to get at, and even more, it's often difficult to discern how to revert the state transition. These are usually small things in the scale of PW (and happen far less than in other frameworks I've worked with) as a whole but will pop up every so often as the codebase evolves and grows. No event system will catch that on its own either;

Are there any examples? I've tried to cover all potentially useful state changes with hooks, even if they aren't actively used. For instance, see the large set of hookable methods at the bottom of the Pages class. You'll see similarly named methods in any core class that deals with state changes (like Fields and Templates for example). Though it's also possible I've misunderstood what you mean. 

[bernhard]

i don't understand most of what you are talking about - BUT: i understand most of what i am doing with processwire

It's not designed for ease of consumption by xdebug, I'll give you that. But it is designed for ease of consumption by you and me, which I consider more important.

thats what i love about processwire! you don't "see" things that you don't understand and therefore don't need to see. that starts with the admin-interface treeview and continues when you start looking to the file system and furthermore into the code. i just wanted to point out that for a non-professional coder like me these are really important benefits! i like the way PW addresses also beginners and at the same time makes it possible to dig deeper into it if you want to and if you are ready! not before.

that said from someone who has created his first hooks some days ago

ps: of course i also love discussions like this one on the forum reading about things that i can maybe think of some day.....

[bfncs]

Thanks for the very elaborated and enlightening answer, Ryan! After you explained the rationale behind the current architecture, a lot of what I was criticizing in the initial post simply makes sense now. As everyone here knows, one of the main points for Processwire is your dedication for the code and the community, and this post certainly is no exception.

I'd still like to go through some of the points mentioned, not at all for proving to be right, but to either underline why the current solution is the best or to explore what improvements might be possible.

The approach described as an alternative here is similar to what I've used on other projects, including PW1. I was really looking to come up with something different for PW2. The goal was to make it as absolutely simple as possible for people to add hooks and for people to make methods hookable. I wanted it to be in the background and everywhere, so that you really didn't even have to think about it.

It's very interesting to know, that you already experimented with other types of event systems and this actually led to the current architecture. Omitting the overhead of a verbose event driven implementation now makes a lot more sense to me than when I initially started this topic and the feedback by others here is approving that this is the right way.

It's not designed for ease of consumption by xdebug, I'll give you that. But it is designed for ease of consumption by you and me, which I consider more important. Admittedly I don't use xdebug very much anymore, so designing things for xdebug has not been a priority.

The big advantage of working with Xdebug – apart from mere debugging – is that it makes you grasp the internal architecture of a piece of software much quicker. This is true especially if you work with a multitude of different frameworks.

Currently, to create a module that extends the core system in a major way (think of something LanguageSupport-related) requires the author to have a very intimate knowledge of the core, the involved hooks and in what context they are called. This is ok and not really disputable, for sure, but since hooks are mostly documented in the source itself, there's no way to get to know all that by extensively reading through implementation details of core modules and hoping to find the right places to do so.

With Xdebug and using step-debugging it should be very easy to sort out the relevant pieces of code, and thereby the hooks that need to be used etc.. If this is easy to do, it might help to get more people into module and possibly core development. The same is true and probably even more relevant for good IDE support.

I think you're right not to prioritize these two aspects, but also they shouldn't be given up too easily. Unfortunately there's no easy way to improve on this, but I think we should at least give it a good thought and look at how other projects get this done.

All the add hook methods are actually handled by just one method: addHook(). The point of the other methods (mentioned below) is to make things simpler for you, and make your code more readable. I prefer verbose method names to boolean and array arguments because they are self describing. think this is a benefit, not a drawback. [...]

Well, I think this is more a matter of preference than anything else, but thanks a lot for laying this out in depth. That made it a lot plausible for me and I don't think that a change is badly needed here.

I would love to make everything hookable, but the reality is that there is a little overhead associated with every hook (in any hook system). So I don't usually make methods hookable unless there is a request for it to be. We started with very few hooks, and now we have quite a lot. But the majority of the hookable methods you see in PW today are the result of people asking for them to be hookable. And luckily, it's very easy to do just that: just append 3 underscores, and you are done. 

I would answer this pretty much the same way as the Hookability point above. When more granularity is needed somewhere, we add it, and there's lots of examples throughout the core. But it's hard to know where one might need that until it is asked for somewhere specific. When a module like ProcessPageEdit was originally written, I honestly didn't know if anyone would ever want to hook anything in it at all. Over time, it has become more granular, and over more time will become even more so I'm sure, but only when specific needs for granularity/hooks come up. I love flexibility, but I also love efficiency. If everything in ProcessWire was fully granular and hookable PW might be very flexible for someone looking to hook specific things, but it would also be bloated and slow. My preference is to make specific things more granular and hookable when it will solve someone's specific need. Granted, there are plenty of hooks in PW already that nobody is using, but they are generally in places where it makes no difference in terms of efficiency. I'm not suggesting that hooks aren't efficient – there's very little overhead from hooks. It's just that if you multiply a very little overhead over thousands or hundreds of thousands of calls, then it becomes something.

All you say here makes sense, but I still think that BitPoet was on to something important when he mentioned hookability, statefulness and granularity as problems – much more than myself actually.

The current procedure of making a method hookable when someone requests it on the forum worked out well until now because there was a manageable number of people developing modules. In my opinion it possibly leads to several problems, though:

• Not everyone is going to request a method made hookable in the forum, especially not, if it's possible to hook anther method with essentially the same outcome – event if it doesn't make sense to hook that particular method semantically. Hooks might be implemented relying on side effects of the hooked method that are more an implementation detail than a stable API.
• These technically working but not really fitting hooks might be broken in further core development without notice. Or they might block further development when the intention is not to break the de-facto API.
• The procedure doesn't scale well from a little developers with a few hook whishes to a lot of developers with a host of hookability requirements. In the end this could lead to a majority of functions being hookable, which – as you mentioned – isn't a good idea performancewise.

In this respect it might make sense to use the fact that there will be breaking changes in 3.0 either way to also revise some parts of the hookable API and try to break it down some more. This should also be a community effort, so it would be needed to discuss some helpful criteria beforehand.

I also see this as an advantage for the event system mentioned in the first post, because it doesn't matter in which method an event is thrown, as long as the documented premises or fulfilled. Thereby it helps with decoupling API from implementation detail and enabling module authors to develop modules without having to dig too deep in the core. But this is also possible without such a structural change and already happens with methods like Pages::___saved(), Pages::___added() etc.

I hope I was able to make some arguments a bit more clear and advance the discussion a bit. I'm still really interested in any feedback.

[horst]

a single point I don't understand:

The same is true and probably even more relevant for good IDE support.

What does it mean, good IDE support?

[bfncs]

What does it mean, good IDE support?

Well, in fact this was worded the wrong way around, for sure it's the IDE that should support things that make it easy to work with a piece of code and not the other way around.

That said, when writing a piece of software the author can make it more or less easy to do static analysis of the code. The information that is gatherable by static analysis enables IDEs to do their "magic" in providing all kinds of convenience when working with code (look at PhpStorm's features to see what this might include). It makes it also more easy to work interactively with a debugger like Xdebug, or code quality tools like PHP Mess Detector, PHP Code Sniffer and others.

All kinds of code that is only available at runtime is not available to static analysis and thereby prevents some features of these tools from working correctly. The most clear example is code executed through eval(), but also magic functions, objects loaded by passing the class as a string argument to a function or functions executed by hooks can make problems here.

I think it makes a lot of sense to prioritize having a concise, human-friendly API over making the code well-consumable by static code analysis. Still it would be great to improve in this sector to benefit as much as possible from the existing tools in the PHP world.

[horst]

Ah, thanks for the explanation.

It is very useful to analyze code or test it against scenarios, also if done staticly. But it also can do a lot of harm or result in the wrong direction.

I give you a real live example about static analysis that happens to me right a few days ago:

Google analyzes websites due to the user experiences with mobile devices. They have sent me a message with their analyzis result where they say that I have to change some things to full fit their requirements, otherwise they don't list those pages in search results requested from mobiles.

One of their usefull tips and requirements is: I should avoid calling a JS-resource in the html head, I should not call external resources above the fold! Otherwise it would be not mobile friendly. (Tada!)

But in fact, the resource I call there is a very little JS file that don't use dependencies, it just checks if the site is viewed by a mobile device. And if it detects a mobile device it sets some things to avoid overhead and loading of to many resources etc. (like limiting infinite adding to only 2 items instaed of 16 with a desktop, fetching way smaller images, adds a mobile class to the body tag to derive the correct mobile friendly CSS styles from it, etc.)

You also should know that all other resources the page needs are: one google-fonts call, one minified css file, and one minified js file which is called just before the body close tag. Also this site uses ProCache and is responding quick like hell.

As I can see, in this case it is pretty stupid and useless to follow some static rules, it results exactly in the opposite direction. (If I would call the mobile checker beneath the fold! it could be that there were already to much items (pre)loaded;  that the content needs to be rebuilded, rearranged according to the to late added mobile class to the body tag; the fetched images were to big, etc. etc) In fact, the whole users experiences, including those on tablets and desktops (not only those on mobiles), depends on calling this little JS early!

If they simply would have built in some logic, it could be a helpful initiative, but how it is used now it is dumb and ignorant.

Edited March 18, 2015 by horst

[teppo]

@Horst: without taking this too far off-topic, hopefully, I couldn't agree more; both about static code analysis in general (I've written multiple lengthy posts about that here and in GitHub, and have a half-finished blog post on the way too) and your real-world example. My opinion is influenced by the fact that I don't use modern IDEs at all, though, so take it with a grain of salt.

Google has apparently increased the weight of those "above the fold" rules, and while they have good intentions, in many cases those have very little to do with actual mobile usability (or speed, for that matter). If it wasn't Google, I'd disregard the whole thing as another well-intending idea hindered by broken implementation, but because their opinion matters a great deal (from search engine traffic point of view), it seems that I'm going to have to make some sites worse than they are now (for humans) in order to make robots happy

[horst]

 ... it seems that I'm going to have to make some sites worse than they are now (for humans) in order to make robots happy

Exactly! That's what I have to do too. (First I will try to trick it by embedding the JS content in the head tag, not linking to it as external resource. If this will not bump up the result, I try to put it 10% beneath the body tag. If it will not bump up, I try to put it 20% beneath, etc. etc.)    (what a bullshit!)

[interrobang]

@boundaryfunctions

Regarding good IDE support I just want to mention that ProcessWire already has Phpdoc-Comments for most of its magic functions. At least in PhpStorm you get all the autocompletion if you tell your IDE which default variables are there. In templates where I want autocompletion I have a comment like this at the beginning of the file:

// =============================================================================
// ProcessWire API Variables:
/** @var $config Config */
/** @var $log WireLog */
/** @var $notices Notices */
/** @var $sanitizer Sanitizer */
/** @var $modules Modules */
/** @var $fieldtypes Fieldtypes */
/** @var $fields Fields */
/** @var $fieldgroups Fieldgroups */
/** @var $templates Templates */
/** @var $pages Pages */
/** @var $permissions Permissions */
/** @var $roles Roles */
/** @var $users Users */
/** @var $user User */
/** @var $session Session */
/** @var $input WireInput */
/** @var $languages Languages */
/** @var $page Page */

// =============================================================================
// Common used aliases:
/** @var $p Page */
/** @var $img Pageimage */

And if you need autocompletion for your custom fields you can use the TemplateStubs module

[bfncs]

Thanks for your feedback!

Horst, I can totally understand your anger about the Google mobile optimization thing. While in the long run their recommendation might be a good idea, this is definitely not helpful at all for at lot of projects at the moment. And especially not, while the picture element isn't something we can rely. However I don't really see how this relates to PHP static code analysis. It's one thing when a quasi-monopolist transnational company tries to force conventions on how you should write markup, but it's another whether users of a piece of software can freely choose whether they want to use a tool or not.

I don't think that anybody would doubt that the right tools have a big influence on how efficient you can work on something, and the right tools are normally the ones you already feel comfortable with. For any piece of software, this means that it's an advantage to allow the usage of as many tools as possible. For sure this should never stand in the way of making an API concise and well consumable for humans. But in my opinion it's worth to consider both.

For example when I work on projects based on Symfony or CakePHP (yep, introducing a slippery slope argument here, since both projects have a fundamentally different scope than Processwire) in my IDE, I can always click on a method call and the file where it is defined is opened. I get autocompletion for classes, methods and variables and I can read the documentation of all of them by merely hovering an occurence. In addition, obvious errors (like reading a variable that has never been declared, calling a non-existing function etc.) are shown with a curly red line under it. All of this (and a lot more) is pretty helpful to me.

I have been working on PHP projects for years without any IDE or debugger, was absolutely satisfied with my technique and thoroughly opposed to the idea of using a heavyweight IDE. Then I discovered PhpStorm, gave it a try and it easily multiplied my work outcome and my grasp of the architecture of frameworks and libraries I used. However, with Processwire – that I love in nearly every other aspect – I have to eschew some of the benefits I was used to. I don't think that everybody should work like I do, to the contrary: everybody should choose his/her own tools by preference. But the current situation excludes some tools that rely on static analysis in the first place.

Well, all I wrote is also slightly offtopic, because there's not really a possibility to reconcile all of this requirements for humans and robots. The only method I can imagine to fulfill all criteria while keeping the great flexibility would be dynamic code generation. And this is in itself creepy enough that I would dare to propose it. In the future the already mentioned runkit extension might be of some help, not for static analysis but for debugging. Otherwise I learned a lot about why the current implementation was chosen in this thread and can now much more appreciate its benefits.

The questions I'm still eager to discuss, though, are how to improve the granularity and the documentation of hooks.

@interrobang: Thanks for pointing out what can be achieved with PhpDoc comments! This is just great and I'm already using all of this and have TemplateStubs installed as the first step in every new PW instance. Works like a charm but because of the dynamic nature of the software, there are still a lot of uncovered areas, especially when working on modules.