Improved class concept

plugin-file.php

<?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();

@rmccue

If you mean the constants, I mentioned above that this would not work.

Why would constants not work? I don't see your mention, but I also don't see why they wouldn't work using reflection to grab the constant's value vs. the DocComment. They would also be more performant than DocComments, i.e. no need for preg_match().

My argument has been against your preferred option (as far as I can tell),

Ok, please stop. This is not a political race where you have to position me. So stop putting words into my mouth.

I have no "preferred" option, both are useful. Implicit is good for rapid prototyping and private use, and Explicit is good for published code.

Which can be worked around without much issue

I really, really wanted the PHPDoc approach to work. But I frankly believe the workaround would be too processing intensive and/or kludgy. My personal opinion only. At this point I'm not sure what a workable solution would be.

In any case, here's the full version, including a static class....

Thanks for the offer to accommodate my concerns, but your use of a base class makes it a non-starter for me. We'll just have to diverge at this point.

@mikeschinkel

Why would constants not work?

Apologies, I actually misread your original comment and came to the same conclusion as @toscho, as I thought the plan was to redefine constants for each method. I see you clarified that in your response, although I missed that on my first reading.

It's certainly possible, and I do see some merit in it, although I think this suffers from the same problem as simply hooking in a method. The advantage of this class from my point of view is the ability to have the hooks with the method itself, so I think you lose some of that with a single constant. Another thought I've had though is that we could have prefixed constants, so WP_ACTION_..., but I think that becomes a bit kludgy. I'm not sure if you can get the full benefits from the constants.

Ok, please stop. This is not a political race where you have to position me. So stop putting words into my mouth.

Apologies, I came off as unnecessarily hostile and that was not my intention at all. I was trying to boil it down to the core to see if we were, as you said, "talking past each other".

Implicit is good for rapid prototyping and private use, and Explicit is good for published code.

I do agree. My issue was that people will forget to switch to explicit when publishing their code, opening it to breakages.

I think that overriding the prefixes however wouldn't have this problem, as it's immediately obvious that you've specified this in the code.

I really, really wanted the PHPDoc approach to work. But I frankly believe the workaround would be too processing intensive and/or kludgy.

I do have to agree that it will be a bit kludgy, but that's one of the side-effects of working around bugs (SimplePie has entire classes dedicated to working around bugs). (I do see this as a bug in eAccelerator, not a feature. Other libraries such as Doctrine rely on PHPDoc, and stripping it has almost no effect on performance.)

To be honest, I'm not sure if it's worth working around, or if it's better to simply tell the user that their installation is broken.

your use of a base class makes it a non-starter for me

I've updated the class, and Rotor_WPPlugin_Base::_register_hooks can now be called publically and without extending.

@rmccue - Thanks, it's all good.

At this point I'm going to go back and focus on Sunrise and it's sponsoring project for a while.

Seen this? I worked on this a while ago: https://gist.github.com/3b79a70d9bf8fa3603f5

I should also add that while I was momentarily excited about the idea of automatic hooks, I've not used them, for the following reasons:

• The names look clunky. My standard was {action|filter}__{name}{_priority(optional)}. So: action__init or filter__the_title_20.
• You have no control over when the hooking happens. The best practice is to "layer" your hooks. Not everything can or should be registered at init or plugins_loaded. This sort of layered hooking should happen in almost every plugin, but it's not possible with automatic hooking.

@markjaquith Of course, you can delay the hook execution, and you don’t need those ugly function names. See https://gist.github.com/1631723 for another example. The only problem now is the availability of PHPDoc comments for the reflection API.
For my use cases it is good enough. For WordPress it has to be more … compatible.

@toscho — I'd rather focus on something that all plugin authors could use, not company-internal code that will never see the light of day.

To that end, I'd rather do something that simplifies hooking, without requiring esoteric PHP setups, strict PHPDoc comments, or clunky method names.

Something like this, for instance:

<?php
class Foo_Plugin extends WP_Plugin {
    public function __construct() {
        $this->add_action( 'init' ); // Hook name only. Vast majority will be this.
    }

    public function init() {
        $this->add_filter( 'the_title', 25 ); // Hook name and priority. Some will be this.
        $this->add_filter( 'the_title', 'the_title2' ); // Hook name and non-standard method name name. Almost never used.
    }

    public function the_title( $title ) {
        return str_replace( 'foo', 'bar', $title );
    }

    public function the_title2( $title ) {
        return str_replace( 'foo2', 'bar2', $title );
    }
}

WP_Plugin::register_plugin( 'Foo_Plugin' );

@markjaquith I like the idea of the variable parameters to add_filter, but I'm also still a fan of the PHPDoc comments. I still think the benefits outweigh the problems. (Especially given that eAccelerator doesn't appear to be supported any longer.)

As per your original implementation, I'm not a fan of having the priority in the name, however it's otherwise identical to my implementation with:

parent::__construct(array('filter' => 'filter__', 'action' => 'action__'));

I agree with not always hooking, but implementing an alternative to that is probably going to be hard (e.g, adding a @wp-add-hook-on plugins-loaded).

Implemented the variable parameters to add_filter/add_action

@markjaquith:

Questions about your proposal:

• 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?
• 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.
• 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?
• What about a plugin that contains multiple classes? Would you envision all classes inherit from WP_Plugin?
• Do you envision core adding class:static_method() APIs in the future that theme developers would need to call vs. the more simplistic the_function_as_template_tag() standard currently provided for theme developers?

Non-standard method name for hooks

You mention in your code comment that they are almost never used, and yes they are rare but I wanted to delineate for others who may not recognize (some of) the places where they are required or preferred:

• Hooks that include filenames such as 'load-plugins.php'.
• Hooks with dashes such as and 'admin_head-$page'.
• Multiple hooks using one function such as one used for 'load-page.php' and 'load-page-new.php'.

Automatic Hooks

As discussion above in the long thread, it would be possible to do automatic hooking on an exclusion basis vs. an inclusion basis.

Your auto-hook example

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?

@mikeschinkel

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?

It's essentially equivalent to all parameters, since no action/filter will ever have that many parameters. It shouldn't cause any problems, but some functions which are hooked in multiple times might need different numbers of parameters for each hook, so that could cause a problem. I do agree though, it should be the default in WordPress.

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

@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.

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

@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.

@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.

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' ) )

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).

@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.

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!

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!

@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.)

@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.

@mikeschinkel

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

Oh. I can haz a vizion?

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!).

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.

@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.

@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

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)