add_action considered harmful

I’m going to argue that add_action is actually a more elaborate version of goto hidden in  plain sight, an anti-pattern, that it encourages unstructured programming, and obscures dependencies between code by providing the illusion of structured code, and encouraging spaghetti architecture.

The Dangers of Goto

goto

Firstly what is a goto statement?

goto (GOTOGO TO or other case combinations, depending on the programming language) is a statement found in many computer programming languages. It performs a one-way transfer of control to another line of code; in contrast a function call normally returns control. The jumped-to locations are usually identified using labels, though some languages use line numbers. At the machine code level, a goto is a form of branch or jump statement. Many languages support the goto statement, and many do not; see language support for discussion.

The goto statement itself was added in PHP 5.3, and has been present in programming for decades. It’s famous for being a notorious omen of bad code. It’s caused problems everywhere from scaling code, to preventing auto-optimising compilers from doing their job.

Computer science has proven that goto like statements are unnecessary to construct  a program via the structured program theorem, and giants of computer science and programming have poured scorn on the goto statement since its inception. Few aspects of coding have become so ingrained as to warrant an XKCD comic warning of their use.

The primary danger faced by users of goto is that it completely bypasses any structure or order in your program by jumping into a completely different context without warning.

When calling do_action(), you’re doing the same thing. Code execution heads to a collection of arbitrary functions that have registered themselves as interested. These callable items may be anywhere in the code, they may call more actions and trigger more events, and they may loop back on each other. Some may not exist in the normal sense as anonymous functions with no means of manipulating them via normal methods.

Coupled with the lack of an abort/termination procedure, and loose controls, this makes the hook system dangerously opaque,  an unstructured wild goose chase of unknowns.

A Note on add_filter

Filtering values is not the same as actioning an event. When passing a value into a filter, your expectation is that the filter modifies the value. Actions however have business logic. They don’t just modify values, they create things and do work, and one should never do work in a filter.

Pitfalls

Anonymous Objects

Consider this code for a moment:

View the code on Gist.

How would you remove that action? The object it’s bound to lives inside the filter system with no other references, so you cannot reconstruct the value passed into add_action. To do so you would need to delve deep into the internal implementation of the filter system.

The ‘all’ hook

Every action/hook fired also fires the ‘all’ action. You can use this to intercept all actions and filters, but in doing so you pay a hefty performance cost.

Structural Deception

It’s generally seen as a good thing to add hooks before and after things happen, and on important events. As useful as this is, it is not a foundation to base software architecture off of, for the same reason that a javascript application composed of hundreds of jQuery( document ).click() style event bindings is frowned upon.

It encourages a sprawling disorganised collection of procedural code, with no incentive to pull common functionality together into common files. Why might this be a problem? Lets take a look at a WordPress callgraph:

WordPress callgraph

Click on the image for the full graph, be warned though, the image is 7k x 9k and 3MB after being ran through photoshop optimisation ( previously 7.5MB PNG-24 ). Here we see a spahghetti bowl architecture. The bold line running from the start to the end is the critical path PHP takes through the program from start to end.

Here’s apply_filters:

apply_filters

We can see here that WordPress core has a choke point at apply_filters consuming as much as 21% of total page load time.

Here is an example of a Drupal callgraph:

Example Drupal callgraph

Far more orderly and structured. What about the Drupal equivalent of hooks and filters? Hooks in Drupal take the form of a function named modulename_hookname. This enforces structure and modularisation. It’s easy to check if a module makes use of a hook, how many pieces of code have a hook, and where to expect a modules entry point for an event will be situated. It could be better but it has advantages. You can read more on Drupal hooks and events here. Note that I am not advocating adoption of the Drupal system.

Another example of a better callbacks and event hooks/actions would be the DOM. While it isn’t a perfect system, one can handle and abort the propagation of an event. Code is bound to an actual location on the DOM rather than arbitrary identifiers, with well defined behavior concerning the flow of an event as it bubbles up the DOM.

Callgraphs can differ depending on the environment they’re ran in, but you can generate your own callgraphs using xhprof.

Loops & Misdirection

Consider this code:

View the code on Gist.

Here we can see an API that provides an action indicating it has done its job. We then add an action to do operations when the action is fired, causing an infinite loop.

While this is a simple case that’s easier to debug, once actions call functions that fire actions, the layering can obscure the program flow. This non-linear flow is confusing and hard to visualise.

Functions may directly interact through a changing tree of callbacks and actions, despite being in unrelated areas of WordPress with no documentation or indication they’re related. This is one of the reasons why disabling all plugins and reverting to the default theme is such an effective debugging tool in WordPress, and that isn’t a good thing.

do_action & Global State

Consider this code providing useful action hooks:

View the code on Gist.

You have written tests to ensure that this function does what it needs to do. When you run your test suite the test passes, however, when you run the test in isolation it fails. What could the issue be?

Consider the reverse situation. The functions performs flawlessly in isolation, but as part of a greater whole, fails miserably. What then?

The problem here is that the functionality of the code has undeclared dependencies in the global state. Things are being set up or initialised during these hooks that are needed for the function to work, but anybody looking at the function would be unaware. It may also be that the expected environment is changed during the hook and the assumptions held no longer apply.

What should happen is that the code should require all of its dependencies up front, such as posts, APIs, and other objects, allowing proper isolation and self documentation. These dependencies can then be intercepted, wrapped, overridden, or manipulated as is necessary.

For example, here is a plugin that separates out its data into a model, that’s then passed through a filter. It can be replaced with a new object implementing the same interface, or a wrapper object that intercepts values as methods are called. This way hooks and filters are unnecessary, yet structure remains intact in a linear, easier to debug way.

Causes

WordPress has a strong incentive to keep backwards compatibility, and this means keeping deprecated functions and being careful with the API. Part of this API is hooks and filters, so they’re never going away entirely, but that isn’t the issue. The issue is that hooks being used as a paradigm for software development are hiding and obscuring the lack of clean high level architecture in WordPress by fooling people into thinking it is a high level architecture.

There are a multitude of basic fundamental design patterns that could be applied to WordPress core with trivial effort and immense long term gains. For example, if global variables were stored in a container object instead of the global namespace, or if files were internally arranged appropriately into modules. These could be done without changing hooks or the function APIs.

New people exposed to this paradigm get caught up in the process, and people believe it is the best way to do things. It may well be that the core developers have no idea that this is the case.

This will only get worse. With hooks being seen as fundamental to the architecture of WordPress, discussions are under way to implement a JavaScript version for the admin panel, despite this being a thoroughly solved issue in the JavaScript ecosystem.

Systems such as backbone and ember.js provide examples of modular code that handle data and events far better than WordPress’ hook system, and the javascript community itself is investing time and effort into improving on their flaws. This effort should not be ignored. What would be best is to document and standardise on a set of common technologies and adopt a paradigm that’s in use elsewhere and proven. Reinventing the wheel and trying to be unique on such a major project is a too high a risk strategy.

In conclusion, I would recommend the following:

  • WordPress style actions and hooks are massively misused, and dangerous.
  • Actions and hooks obscure program flow making debugging and design hard to manage
  • Choke points are introduced, making architectural decisions difficult, and introducing unnecessary complexity
  • WP Core code complexity can be simplified in a backwards compatible way by instead relying on good OO design, modularisation, and use of good design patterns, such as wrappers, encapsulation, and other means.
  • JS code should inherit the well studied and applied lessons of the JS frameworks, rather than reinvent a failed paradigm in a new language

There is no magic fix going forward, but actions and hooks are neither healthy or necessary.

10 thoughts on “add_action considered harmful

  1. Nice post and very interest for me. Thanks.

    For debugging, a small hint. I use my plugin Debug Objects and have a mode inside, there deactivate the theme and plugins, to use the defaults and little bid easier debugging of my development.

  2. The overall concept might be questionable, yes ( but Backbones Events can go insane as well ), but if you use xhprof to demonstrate certain aspects you should add some more context / explanation to it.

    The “insanity” of the callgraph heavily depends on theme/plugin configuration, same applys to the wall time of “apply_filters”, and of course, one should consider the called operations / functions.

    I don’t think it’s fair to take one/two screenshots just to substantiate your original point, as long as the context of those screenshots / profile is missing.

    • I agree, call graphs can shift from load to load, a tutorial on how to produce them is really the only additional thing I could have put there to ellaborate on it. That being said, it’s the use of hooks that allows it to become so insane, and call graphs were just the most visual method of demonstrating it in what’s otherwise a very wordy post

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.