add_filter should accept an array of tags

[sivel]

Currently when you want to hook a single callback into multiple filters/actions you create an array and loop through it calling add_action/filter each time. It would reduce code and make it easier to do this by just having add_filter accept an array of strings as it's first argument. In addition tests have shown that it also improves overall performance of WP by using this.

I talked to nacin on IRC recently about this and he was concerned about negative performance impacts that this may have, since most of our add_action/filter calls are for single tags. I did some performance benchmarking and found the following results:

With having add_filter accept an array it was 0.000002002716 seconds slower for single actions, but 0.000007534 seconds faster for a 4 action loop. Without add_filter accepting an array each iteration of a foreach in the way we are currently handling it takes 0.00000885725 seconds, accepting an array each iteration takes 0.00000697374 seconds. Which is a 0.000001884 improvement during add_action/filter iterations.

Overall on a default install of WP 3.1-alpha this modification actually improved performance by 0.00048398971558 seconds faster by having add_filter handle all of the looping.

Anyway, this is an early patch and likely could use a bit of testing and independent benchmarking.

At the moment I have not added this functionality to remove_action/filter and am not really sure we need it.

Please provide feedback, test and provide your own benchmarks.

[scribu]

Something similar has been proposed before: #8640

[scribu]

Marked #21591 as dup.

I don't see this:

add_action( array( 'sanitize_post', 'sanitize_user' ), 'my_sanitizer' );

as a significant enough improvement over this:

foreach ( array( 'sanitize_post', 'sanitize_user' ) as $hook ) {
  add_action( $hook, 'my_sanitizer' );
}

It's the kind of syntactic sugar that makes you fat: tasty, but low in nutritional value.

[joehoyle]

If you are using closures, this isn't just syntactic sugar.

foreach ( array( 'sanitize_post', 'sanitize_user' ) as $hook ) {
  add_action( $hook, function() { return $foo; } );
}

This will create 2 closure objects, whereas add_action supporting an array would only require 2 references to the same closure. (less memory usage, shared scope etc)

You could always do

$func = function() { return $foo };

foreach ( array( 'sanitize_post', 'sanitize_user' ) as $hook ) {
  add_action( $hook, $func );
}

But that feels like it starts to get messy. I would rather create my own add_actions function than do the above, so I don't think it's a stretch to just add the functionality to add_action

[scribu]

Then create your own add_actions() helper; nothing wrong with that. Just don't force it on everyone else that might not need it.

[joehoyle]

It would be a silent addition to add_action, of course you wouldn't have to pass an array. But more importantly I am not trying to make anyone, of course I do create my own functions. I don't submit ideas to trac to force other people to code the way I do, but rather if I find it useful then other people may do too, and it may add value for WordPress core. sheesh.

[nacin]

PHP is not JavaScript, but even in JS, you assign these things to variables when it makes sense to do so. Closures are nice for one-off things. This is no longer so simple once you require adding it to multiple hooks. Why not a top-level function?

Also of note, if you do use a closure but don't assign it to an accessible variable, it becomes impossible to remove that action later without calling remove_all_actions(). That is lame, and makes closures little suited beyond testing and internal implementations. For distributed work, they are best avoided.

I don't have a specific objection to this, and PHP 5.4 [ ] syntax makes it somewhat elegant. But it does complicate an API that is, at the moment, quite simple.

[scribu]

It would be a silent addition to add_action, of course you wouldn't have to pass an array.

That's not what I meant.

I don't submit ideas to trac to force other people to code the way I do, but rather if I find it useful then other people may do too, and it may add value for WordPress core. sheesh.

I am not accusing you of anything. Just saying that it's not actually that useful, so it should remain a utility that devs can implement in a few seconds, as needed.

[joehoyle]

Replying to nacin:

Yeah, totally agree they have certain applications, though I personally think they are slightly more useful that you say above. IMO the best time to use them is something like:

add_action( 'init', function() {
    register_post_type( ... );
} );

So many plugins do the above with add_action( 'init', 'my_plugin_init' ); that doesn't make sense to be a function. It's not reusable, it's not meant to be added / remove by third parties, so probably shouldn't be added as a top level function. Without closures you end up loads of once-use weirdly named internal functions for callbacks, making it much more difficult to have the codebase coherent.

For distributed code, totally get the remove_action problem, but 95% of the code I write is for internal implementations.

[nacin]

Replying to joehoyle:

So many plugins do the above with add_action( 'init', 'my_plugin_init' ); that doesn't make sense to be a function. It's not reusable, it's not meant to be added / remove by third parties, so probably shouldn't be added as a top level function. Without closures you end up loads of once-use weirdly named internal functions for callbacks, making it much more difficult to have the codebase coherent.

So, I certainly see where this sentiment is coming from, and am inclined to agree. But I'm not sure it scales well beyond a single hook. Once you need to attach the same callback/closure to multiple hooks, I think that's generally a really good indicator that we've crossed (or at least approached) the point of reusability.

[dd32]

#26704 was marked as a duplicate.

[swissspidy]

#35556 was marked as a duplicate.

[SergeyBiryukov]

#40881 was marked as a duplicate.

[SergeyBiryukov]

#48824 was marked as a duplicate.