Skip to content

Instantly share code, notes, and snippets.

@rmccue
Forked from kovshenin/plugin-file.php
Created January 17, 2012 12:27
Show Gist options
  • Save rmccue/1626492 to your computer and use it in GitHub Desktop.
Save rmccue/1626492 to your computer and use it in GitHub Desktop.
Improved class concept
<?php
/*****
All new versions will be posted at
https://github.com/rmccue/Rotor_WPPlugin
Please use that repository instead of this Gist.
******/
/*
Plugin Name: WP_Plugin class test
Description: A better test!
Author: Ryan McCue
Version: 1.0
Author URI: http://ryanmccue.info/
*/
class WP_Plugin {
public function __construct() {
$self = new ReflectionClass($this);
foreach ($self->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
$params = $method->getNumberOfParameters();
$hooks = array('filter' => array(), 'action' => array());
if (strpos($method->name, 'filter_') === 0) {
$hook = substr($method->name, 7);
$hooks['action'][$hook] = 10;
}
elseif (strpos($method->name, 'action_') === 0) {
$hook = substr($method->name, 7);
$hooks['action'][$hook] = 10;
}
else {
$doc = $method->getDocComment();
if (empty($doc) || (strpos($doc, '@wordpress-filter') === false && strpos($doc, '@wordpress-action') === false)) {
continue;
}
preg_match_all('#^\s+\*\s*@wordpress-(action|filter)\s+([\w-]+)(\s*\d+)?#im', $doc, $matches, PREG_SET_ORDER);
var_dump($matches);
if (empty($matches)) {
continue;
}
foreach ($matches as $match) {
$type = $match[1];
$hook = $match[2];
$priority = 10;
if (!empty($match[3])) {
$priority = (int) $match[3];
}
$hooks[$type][$hook] = $priority;
}
}
var_dump(__LINE__, $hooks);
foreach ($hooks['filter'] as $hook => $priority) {
add_filter($hook, array($this, $method->name), $priority, $params);
}
foreach ($hooks['action'] as $hook => $priority) {
add_action($hook, array($this, $method->name), $priority, $params);
}
}
}
}
// Here's an example of a plugin that would benefit from the above
class Some_Plugin extends WP_Plugin {
// Will filter the_title
public function filter_the_title( $title ) {
return $title . '123';
}
// Will filter the_content
public function filter_the_content( $content ) {
return $content . ' Add more content.';
}
// Will run during wp_footer
public function action_wp_footer() {
echo "I'm in the footer!";
}
/**
* @wordpress-action init
* @wordpress-action admin_init 25
*/
public function my_init() {
echo "I'm in the footer!";
}
}
$some_plugin = new Some_Plugin();
@mikeschinkel
Copy link

@rmccue - Thanks. I'm also curious about the political reason for not having it in core in addition to any technical reason.

@markjaquith
Copy link

@mikeschinkel

Very basic implementation here: https://gist.github.com/1645537

Do you propose this syntax: $this->add_filter( 'the_title', 'the_title2', 25 ); when a plugin needs hookname <> method name and when you need priority?

Yes. Or you can flip the priority and method name parameters. As one is always a string and the other is always an integer, we can easily tell what was intended. This allows you to leave off the one you don't need to provide (method name with default priority, or priority with default method name).

How would you propose support when static methods are used for hooks? This is a preferred architecture for some plugin developers including myself because it's easier to remove hooks and doesn't require an otherwise useless instance to hang around in memory.

Not really interested in that pattern. You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data. And if it does, there's no reason to worry about a memory footprint. Would rather just go with one pattern rather than go "oops, gotta hold on to some data here... better change this to an instance now!"

How do you envision themes that need to implement hooks using the proposed WP_Plugin class that want to use a class to limit public name usage in namespace for their themes? Would themes also use WP_Plugin, or would you introduce a WP_Theme class that supports the same methods?

I might just have WP_Theme extend from WP_Plugin, and then have themes extend from WP_Theme.

What about a plugin that contains multiple classes? Would you envision all classes inherit from WP_Plugin?

Depends. If they want to trigger things in the main plugin instance, they should have an interface to the main plugin... some sort of global plugin factory that gives you the instance. If they want to use WP_Plugin methods, then they should inherit from it. At a certain point, a plugin is so complex that someone can just design their own personalized convenience wrapper.

I noticed you used 99 as number of parameters for all hooks. I have not explored that, does it not cause problems? And if not, why doesn't WordPress core use it?

We don't use it in WordPress core because built-in PHP functions cannot be passed more parameters than they accept without causing problems. The built-in PHP functions aren't build with PHP, so they don't behave the same way. It was discussed in this ticket. http://core.trac.wordpress.org/ticket/14671 I proposed an idea, but didn't really like it, and it's just sat there.

5ubliminal had some interesting comments towards the end about using reflection which might be worth investigating.

One other closing thought: do not underestimate how mind-meltingly confusing it is to someone reading your code when a comment changes how the code works.

@markjaquith
Copy link

You should also check out @scribu's wp-scb-framework.

@rmccue
Copy link
Author

rmccue commented Jan 20, 2012

@markjaquith

You should also check out @scribu's wp-scb-framework.

Oh cool, wish I'd known about that before. Looks very similar in terms of hooking ability, although the PHPDoc tags aren't prefixed.

Not really interested in that pattern. You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data. And if it does, there's no reason to worry about a memory footprint. Would rather just go with one pattern rather than go "oops, gotta hold on to some data here... better change this to an instance now!"

That can be stored in static properties if needed. Using objects is an antipattern in the WordPress plugin community that I'd very much like to see die.

5ubliminal had some interesting comments towards the end about using reflection which might be worth investigating.

See https://github.com/rmccue/Rotor_WPPlugin/blob/master/plugin.php#L48 - Works fantastically.

One other closing thought: do not underestimate how mind-meltingly confusing it is to someone reading your code when a comment changes how the code works.

I think that's a good point. Most people do see it in terms of being ignored, but I think there's a certain familiarity with it from plugin headers. In any case, it's certainly worth noting in a plugin class how methods are hooked in.

@mikeschinkel
Copy link

@markjaquith - Thanks for your quick reply.

Not really interested in that pattern.

That's disheartening to learn, at least if you are considering this for inclusion in core.

You have to store your ephemeral instance data somewhere. It's rare for an instance to have no data.

I find the static pattern works better than the instance pattern. For example I have a set of plugins that have over 50 classes where most use static methods and its rare for any of them to have any ephemeral data. Yes there are classes that are used for instances, but they are typically not coupled to WordPress and thus don't implement hooks using their methods.

We don't use it in WordPress core because built-in PHP functions cannot be passed more parameters than they accept without causing problems.

Gotcha, thanks. That clarifies why for methods in a class it wouldn't be an issue.

You should also check out @scribu's wp-scb-framework.

What specifically about it? It seems rather simple.

@markjaquith
Copy link

That can be stored in static properties if needed. Using objects is an antipattern in the WordPress plugin community that I'd very much like to see die.

How are they appreciably different? I mostly see it as a syntactic difference:

  • $this->foo vs self::$foo
  • $this->bar() vs self::bar()
  • add_action( 'foo', array( $this, 'foo' ) ) vs add_action( 'foo', array( __CLASS__, 'foo' ) )

@rmccue
Copy link
Author

rmccue commented Jan 20, 2012

How are they appreciably different? I mostly see it as a syntactic difference:

new PluginClass(); is a misuse of PHP objects. Objects should be used when you actually need objects, not simply for grouping methods together (for that, the best option is a namespace, but static methods in a class is good enough). You'll only ever have one copy of your plugin in memory, so having the ability to create a new instance has no use.

In addition, $myplugin = new PluginClass(); is using global variables, which is another antipattern I hate (especially when it's for no reason).

@mikeschinkel
Copy link

@markjaquith - I concur with @rmccue on his points and will add that it's much easier to remove a hook with a static method than an instance method. You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.

@markjaquith
Copy link

That's disheartening to learn, at least if you are considering this for inclusion in core.

We're not considering it for core at this time. I was just talking in terms of "if I were going to write my own plugin framework...". We've been talking about plugin frameworks for years, and I'm happy to let people go in a bunch of directions with it independent of core and see what happens. The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.

I find the static pattern works better than the instance pattern.

Works better how? Honestly asking here. I've always seen it as a syntactic difference, and I just have a (weak) preference for the instance pattern. Skinny arrows look nicer than a T_PAAMAYIM_NEKUDOTAYIM to me. :-)

What specifically about it? It seems rather simple.

Just some prior art. And I always find @scribu's code fascinating to read!

@markjaquith
Copy link

Ah, we cross-posted.

In addition, $myplugin = new PluginClass(); is using global variables

Well, that is its own issue, and certainly something to discourage.

Objects should be used when you actually need objects, not simply for grouping methods together (for that, the best option is a namespace, but static methods in a class is good enough). You'll only ever have one copy of your plugin in memory, so having the ability to create a new instance has no use.

I can see that argument... but that's still a bit of a stylistic observation.

I concur with @rmccue on his points and will add that it's much easier to remove a hook with a static method than an instance method. You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.

Aha. This is a solid objection (get it? objection? — ::crickets::). I've always stashed the instance in Class_Name::$instance to allow for people to hook in, but that isn't as obvious as just using the class name. Okay, you've given me something to think about!

@rmccue
Copy link
Author

rmccue commented Jan 20, 2012

@mikeschinkel:

You only need the class name to remove the hooked static method but you need the actual instance to remove the hooked instance method.

Great point.

@markjaquith:

Works better how? Honestly asking here. I've always seen it as a syntactic difference, and I just have a (weak) preference for the instance pattern. Skinny arrows look nicer than a T_PAAMAYIM_NEKUDOTAYIM to me. :-)

"Works better" meaning "works better with my mindset". Using an object (to me) makes no sense, because there's no sense of differentiation between objects. If I make two PluginClass objects, what's the difference? Compare to the WP_User class, which is a correct use of objects.

My mindset is always to default to static classes and only use objects if necessary, rather than the other way around.

Well, that is its own issue, and certainly something to discourage.

The problem is that one leads to another. I also don't particularly like the look of new PluginClass without assignment, it looks ugly to me. :)

I've always stashed the instance in Class_Name::$instance

So, you're using the singleton pattern? That's another thing I hate (although, I have used them where it made sense). Singletons are essentially just a reimplementation of static classes, since you go from Class_Name::method() to Class_Name::$instance->method()

The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.

This can be a problem, but it's simply a case of keeping backwards and forwards compatibility.

This is a solid objection (get it? objection? — ::crickets::)

slow clap

(I get the feeling this all deserves a proper blog post rather than discussion in comments.)

@mikeschinkel
Copy link

@markjaquith -

We're not considering it for core at this time.

Cool then.

The only real difference between putting it in core and having it be a third party thing is that as a third party thing you have to take care of versioning (i.e. WP_Plugin_v34) so that plugin A which expects version 34 doesn't get served version 22 of the framework class.

Yeah, versioning is a major PITA, one we are trying to figure out a best practice for now. And I really do not like having to embed the version number into the class name.

We especially need it for loading shared libraries within clients plugins. For example we have two plugins, one for Client A and another for Client B and we want to use in both client's plugins our own (for example) whizbang.php library that is not a plugin (plugins seem best suited for things an end-user can and should control, and putting the dependency into mu-plugins is not viable for a plugin distributed via wordpress.org to end-users.) Best we can come up with it a registration system that will only load the latest version if the two plugins have different versions of whizbang.php. Then it will be up to us to ensure backward compatibility with new releases.

Any chance of getting something into core that can help multiple plugins share the same library? Essentially a standard feature that would allow a plugin to call a function like register_library( $name, $filename, $version ) and then load the latest one after 'plugins_loaded' hook?

(static pattern) Works better how? Honestly asking here.

The biggest reasons were already stated but here they are again, and two more:

  • No unnecessary instance lying around, especially one that might otherwise get stored in a global variable.
  • It's easier to remove static method hooks.
  • It enforces the notion that hooks are a singular thing, not one per instance. With instance methods people who don't understand the architecture might include the hooks in classes designed for creating numerous instances and thus the hooks get triggered multiple times, potentially with negative consequences. That won't happen if the static pattern is used.
  • Least important, a personal preference is to avoid recommending __construct() to would-be plugin developers because the leading underscores make it look more cryptic to the themers who are already afraid of PHP and people might forget to call parent::__construct() if they create their own __construct() in their plugin's class. I always create a static method on_load() and that's where I hook 'init' and potentially others.

Just some prior art. And I always find @scribu's code fascinating to read!

I've always respected your opinion on WordPress coding since my very first WordPress-related project was working with you so I'm hopeful when we release Sunrise to open beta you will find its code similarly fascinating. Given your public comments regarding the future of WordPress I think you are the core team member who will be most likely to appreciate what we've been building.

@thefuxia
Copy link

@mikeschinkel

Any chance of getting something into core that can help multiple plugins share the same library?

Oh. I can haz a vizion?

screen shot

BTW I prefer real instances because it is easier to extend such classes. Yes, PHP now inherits static methods too, but you still cannot rely on it. As for removing filters/actions: If you want to allow this, you can store the object in a registry (WP still hasn’t one) or in a global variable (bah!).

@GaryJones
Copy link

One trouble with using methods that are named in a certain way, is that you can't have two methods hooked to the same hook with the same priority - for example, scripts() and styles() methods hooked to wp_enqueue_scripts that you may want to be individually unhookable or only conditionally hook able depending if a user wants to not use plugin styles, etc.

@rmccue
Copy link
Author

rmccue commented Jan 20, 2012

@toscho:

BTW I prefer real instances because it is easier to extend such classes. Yes, PHP now inherits static methods too, but you still cannot rely on it.

Yes you can, PHP has always inherited static methods. The only problem with it was that self was always bound to the class it was defined in. With late static binding, you can use static instead, but that sucks.

You'll notice in my add_filter/add_action methods for the static class, it works around this for PHP <5.3

@GaryJones:

you can't have two methods [...] that you may want to be individually unhookable or only conditionally hook able depending if a user wants to not use plugin styles, etc

That's one place where you definitely need code, so I agree with Mark's sentiment on making hooking itself easier.

@peterchester
Copy link

@mikeschinkel

We recently made a library management class for our premium plugins since they often share code. We went off the assumption that we'll want to use the latest version since we can't load the same class name at different versions.

https://gist.github.com/2134425

@easterncoder
Copy link

I find myself hating the process of having to declare add_action, etc far away from the method they're calling and so I search big old google and found this.

Was surprised to find that your style of declaring method names so that the action, priority, etc can be parsed for add_action, etc is similar to what I did years before. I guess great minds think alike. I used said method for one of our company's internal WP plugins and it's still running now.

What I do not like about it is the fact that your method names will always look ugly.

I then saw your approach of using Reflection to get the doc comment which is smart and I almost went that route. Two reasons however prevented me from doing so

  1. It's easy to break. Some coder might mess it up and the code will still run without errors since it's only a comment.
  2. Optimizers like eAccelerator can mess it up as well as previously noted in one of the comments.

But that inspired me to think further and asked... what if it's not a comment? What if it's actual code that is declared in the method itself? I thought why not use static variables. A static variable named a certain way so we can easily find it with Reflection the same way you did with the doc comment.

Here's what I came up with:
https://github.com/easterncoder/wp_autohooks

Would love to hear any feedback.

(I can go to sleep in peach now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment